From c49de2a2577cee9399a47d307630e4e32727bdc8 Mon Sep 17 00:00:00 2001 From: Dylan O'Neill Date: Wed, 9 Sep 2026 20:57:08 -0700 Subject: [PATCH] Report criterion scores and let LaunchDarkly rule on them 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 --- .../evaluations/__init__.py | 3 +- .../evaluations/criteria.py | 34 +++- .../evaluations/events.py | 32 +-- .../evaluations/module.py | 11 +- .../evaluations/runner.py | 47 ++--- .../evaluations/types.py | 1 - packages/client/tests/test_evaluations_run.py | 183 ++++++++++++++++-- 7 files changed, 235 insertions(+), 76 deletions(-) diff --git a/packages/client/src/launchdarkly_ai_server/evaluations/__init__.py b/packages/client/src/launchdarkly_ai_server/evaluations/__init__.py index b110a55..bd3eb0a 100644 --- a/packages/client/src/launchdarkly_ai_server/evaluations/__init__.py +++ b/packages/client/src/launchdarkly_ai_server/evaluations/__init__.py @@ -9,7 +9,7 @@ Transport, urllib_transport, ) -from .criteria import Criterion, Judge, Scorer +from .criteria import Criterion, Judge, Scorer, SuccessDirection from .module import EvaluationsModule, init_evaluations from .types import DatasetRow, EvalRunResult, GenerationConfig, RunSummary, Usage @@ -27,6 +27,7 @@ "LDApiError", "RunSummary", "Scorer", + "SuccessDirection", "Transport", "Usage", "init_evaluations", diff --git a/packages/client/src/launchdarkly_ai_server/evaluations/criteria.py b/packages/client/src/launchdarkly_ai_server/evaluations/criteria.py index f77d641..94acd8e 100644 --- a/packages/client/src/launchdarkly_ai_server/evaluations/criteria.py +++ b/packages/client/src/launchdarkly_ai_server/evaluations/criteria.py @@ -2,12 +2,20 @@ from collections.abc import Awaitable, Callable from dataclasses import dataclass -from typing import Any +from typing import Any, Literal from .types import DatasetRow type ScorerFn = Callable[[DatasetRow, Any], float | bool | Awaitable[float | bool]] +type SuccessDirection = Literal["higher_is_better", "lower_is_better"] + +#: The threshold a judge is held to when its reference sets none. A criterion +#: with no threshold gives LaunchDarkly nothing to compare a score against, so +#: it would be recorded and never ruled on; defaulting is what keeps a judge +#: usable as a gate without every caller restating the obvious. +DEFAULT_JUDGE_THRESHOLD = 0.5 + @dataclass(frozen=True) class Judge: @@ -16,10 +24,15 @@ class Judge: The SDK does not create or provide built-in judges. Pass the key of a judge that exists in LaunchDarkly. Resolution uses LaunchDarkly flag delivery for the currently served variation. + + A judge's success direction is not set here. It lives on the judge's AI + Config as ``isInverted`` and is injected onto the criterion by LaunchDarkly + when the evaluation is created, so the one input a verdict is ruled on stays + server-attested even though the score beside it is client-reported. """ key: str - threshold: float | None = None + threshold: float | None = DEFAULT_JUDGE_THRESHOLD pass_rate_threshold: float | None = None ground_truth_context: str | None = None @@ -41,7 +54,12 @@ def to_criteria_wire(self) -> dict[str, Any]: pass_rate_threshold=self.pass_rate_threshold, ground_truth_context=self.ground_truth_context, ) - return {"criterionType": self.criterion_type, "options": options} + return { + "criterionType": self.criterion_type, + "kind": "judge", + "judgeKey": self.key, + "options": options, + } @dataclass(frozen=True) @@ -57,12 +75,20 @@ class Scorer: ``threshold`` defaults to 1.0: a row passes only on a perfect score, which matches the common case of boolean scorers. Pass a lower threshold for graded numeric scorers. + + ``success_direction`` says which way the score points, and defaults to + higher-is-better. Unlike a judge, a scorer has no LaunchDarkly-side config + to read a direction from, so the declaration here is the only source -- + LaunchDarkly derives each row's verdict by comparing score to threshold in + this direction. Set ``"lower_is_better"`` for a scorer that counts + something unwanted, e.g. a regex hit count or an edit distance. """ name: str fn: ScorerFn threshold: float | None = 1.0 pass_rate_threshold: float | None = None + success_direction: SuccessDirection = "higher_is_better" def __post_init__(self) -> None: if not isinstance(self.name, str) or not self.name.strip(): @@ -81,6 +107,8 @@ def criterion_type(self) -> str: def to_criteria_wire(self) -> dict[str, Any]: return { "criterionType": self.criterion_type, + "kind": "scorer", + "successDirection": self.success_direction, "options": _criteria_options( threshold=self.threshold, pass_rate_threshold=self.pass_rate_threshold, diff --git a/packages/client/src/launchdarkly_ai_server/evaluations/events.py b/packages/client/src/launchdarkly_ai_server/evaluations/events.py index cf01ec3..4098e61 100644 --- a/packages/client/src/launchdarkly_ai_server/evaluations/events.py +++ b/packages/client/src/launchdarkly_ai_server/evaluations/events.py @@ -5,28 +5,16 @@ from typing import Any -class EvaluationStatus(StrEnum): +class CriterionStatus(StrEnum): COMPLETE = "COMPLETE" ERROR = "ERROR" -class EvaluationEventKind(StrEnum): +class CriterionEventKind(StrEnum): JUDGE = "judge" SCORER = "scorer" -class EvaluationVerdict(StrEnum): - """Pass/fail outcome for one criterion result on one row. - - Computed by the SDK, not the server: for a judge, the SDK compares score - against threshold using the judge's own direction (isInverted) before this - verdict is ever put on the wire. - """ - - PASS = "pass" - FAIL = "fail" - - @dataclass(frozen=True) class TokenUsage: """Token usage reported by an LD Judge provider call.""" @@ -42,7 +30,7 @@ def to_wire(self) -> dict[str, int]: @dataclass(frozen=True, kw_only=True) -class EvaluationEventPayload: +class CriterionEventPayload: """Common fields emitted for every SDK-run evaluation criterion result.""" project_key: str @@ -52,18 +40,17 @@ class EvaluationEventPayload: dataset_id: str row_index: int criterion_type: str - kind: EvaluationEventKind + kind: CriterionEventKind event_id: str emitted_at: str evaluation_key: str dataset_key: str - status: EvaluationStatus + status: CriterionStatus started_at: str evaluated_at: str latency_ms: int evaluation_version: int | None = None score: float | None = None - verdict: EvaluationVerdict | None = None reason: str | None = None error: dict[str, Any] | None = None error_message: str | None = None @@ -88,7 +75,6 @@ def to_track_payload(self) -> dict[str, Any]: "evaluatedAt": self.evaluated_at, "latencyMs": self.latency_ms, "score": self.score, - "verdict": self.verdict.value if self.verdict is not None else None, "reason": self.reason, "error": self.error, "errorMessage": self.error_message, @@ -97,10 +83,10 @@ def to_track_payload(self) -> dict[str, Any]: @dataclass(frozen=True, kw_only=True) -class LDJudgeEvaluationEventPayload(EvaluationEventPayload): +class LDJudgeCriterionEventPayload(CriterionEventPayload): """Payload for one LaunchDarkly AI Judge result on one dataset row.""" - kind: EvaluationEventKind = EvaluationEventKind.JUDGE + kind: CriterionEventKind = CriterionEventKind.JUDGE judge_key: str variation_key: str version: int | None = None @@ -118,7 +104,7 @@ def to_track_payload(self) -> dict[str, Any]: @dataclass(frozen=True, kw_only=True) -class DeterministicScorerEvaluationEventPayload(EvaluationEventPayload): +class DeterministicScorerCriterionEventPayload(CriterionEventPayload): """Payload for one local deterministic scorer result on one dataset row.""" - kind: EvaluationEventKind = EvaluationEventKind.SCORER + kind: CriterionEventKind = CriterionEventKind.SCORER diff --git a/packages/client/src/launchdarkly_ai_server/evaluations/module.py b/packages/client/src/launchdarkly_ai_server/evaluations/module.py index 57a7752..143036c 100644 --- a/packages/client/src/launchdarkly_ai_server/evaluations/module.py +++ b/packages/client/src/launchdarkly_ai_server/evaluations/module.py @@ -215,7 +215,16 @@ async def run( f"{_segment(evaluation.id)}/runs/{_segment(evaluation_run.id)}" ) return EvalRunResult( - passed=(summary.error_rows == 0 and summary.pending_rows == 0), + # failed_rows counts rows whose criteria were scored and did not + # meet their threshold, so a gate that ignores it exits 0 on a run + # where every row failed its judge. It was omissible while runs were + # generation-only -- a row either generated or errored, and nothing + # produced a fail -- and stops being so the moment criteria exist. + passed=( + summary.error_rows == 0 + and summary.failed_rows == 0 + and summary.pending_rows == 0 + ), url=url, run_id=evaluation_run.id, summary=summary, diff --git a/packages/client/src/launchdarkly_ai_server/evaluations/runner.py b/packages/client/src/launchdarkly_ai_server/evaluations/runner.py index a93e61d..73c18a3 100644 --- a/packages/client/src/launchdarkly_ai_server/evaluations/runner.py +++ b/packages/client/src/launchdarkly_ai_server/evaluations/runner.py @@ -26,11 +26,10 @@ from .api import EvaluationsError, LDApiClient, LDApiError from .criteria import Criterion, Judge, Scorer from .events import ( - DeterministicScorerEvaluationEventPayload, - EvaluationEventPayload, - EvaluationStatus, - EvaluationVerdict, - LDJudgeEvaluationEventPayload, + CriterionEventPayload, + CriterionStatus, + DeterministicScorerCriterionEventPayload, + LDJudgeCriterionEventPayload, TokenUsage, ) from .types import ( @@ -48,7 +47,7 @@ DATASET_PAGE_SIZE = 200 GENERATION_EVENT_NAME = "$ld:ai:offline-evals:generation" -EVALUATION_EVENT_NAME = "$ld:ai:offline-evals:evaluation" +CRITERION_EVENT_NAME = "$ld:ai:offline-evals:criterion" EvalHandler = Callable[..., Awaitable[dict[str, Any]]] ToolImplementation = Callable[..., Any] | NativeTool @@ -184,11 +183,6 @@ async def _resolve_judges( version=int(meta["version"]) if isinstance(meta.get("version"), int) else None, - # Absent (e.g. Gonfalon hasn't deployed the flag-payload change yet, or a - # custom judge with no direction set) resolves to None, not a raised error. - is_inverted=config.get("isInverted") - if isinstance(config.get("isInverted"), bool) - else None, ) return resolved @@ -733,23 +727,17 @@ async def _run_ld_judge_for_result( "invalid_score", f"judge score must be a number between 0 and 1, got {raw_score!r}", ) - verdict: EvaluationVerdict | None = None - if judge.threshold is not None: - # An unresolved direction (e.g. Gonfalon hasn't deployed the flag-payload - # change yet, or a custom judge with none set) defaults to upper-is-better, - # matching Gonfalon's own default for a judge with no stored isInverted. - passed = ( - score <= judge.threshold - if resolved.is_inverted - else score >= judge.threshold - ) - verdict = EvaluationVerdict.PASS if passed else EvaluationVerdict.FAIL completed = datetime.now(UTC) + # No verdict: the SDK reports the score and LaunchDarkly rules on it. The + # criterion carries the threshold and the judge's success direction, and + # ai-evaluator compares them at ingest -- so pass/fail policy is one + # server-side implementation that applies to every SDK version and to runs + # already recorded, rather than one frozen into each release of each + # language's SDK. event = { **base, "status": "COMPLETE", "score": score, - "verdict": verdict, "reason": reason, "evaluated_at": completed.isoformat().replace("+00:00", "Z"), "latency_ms": round((time.perf_counter() - started_clock) * 1000), @@ -859,19 +847,18 @@ def _emit_evaluation_events( "evaluation_key": evaluation.key, "evaluation_version": evaluation.version, "dataset_key": dataset.key, - "status": EvaluationStatus(result["status"]), + "status": CriterionStatus(result["status"]), "started_at": result["started_at"], "evaluated_at": result["evaluated_at"], "latency_ms": result["latency_ms"], "score": result.get("score"), - "verdict": result.get("verdict"), "reason": result.get("reason"), "error": error, "error_message": error_message, } - payload_model: EvaluationEventPayload + payload_model: CriterionEventPayload if result["kind"] == "judge": - payload_model = LDJudgeEvaluationEventPayload( + payload_model = LDJudgeCriterionEventPayload( **common_payload, judge_key=result["judge_key"], variation_key=result["variation_key"], @@ -879,11 +866,11 @@ def _emit_evaluation_events( usage=usage, ) else: - payload_model = DeterministicScorerEvaluationEventPayload( + payload_model = DeterministicScorerCriterionEventPayload( **common_payload ) client.track( - EVALUATION_EVENT_NAME, context, payload_model.to_track_payload(), 1 + CRITERION_EVENT_NAME, context, payload_model.to_track_payload(), 1 ) except Exception: logger.exception( @@ -894,7 +881,7 @@ def _emit_evaluation_events( continue logger.info( "%s emittedAt=%s eventId=%s", - EVALUATION_EVENT_NAME, + CRITERION_EVENT_NAME, emitted_at, event_id, ) diff --git a/packages/client/src/launchdarkly_ai_server/evaluations/types.py b/packages/client/src/launchdarkly_ai_server/evaluations/types.py index 057a37e..b5d0e64 100644 --- a/packages/client/src/launchdarkly_ai_server/evaluations/types.py +++ b/packages/client/src/launchdarkly_ai_server/evaluations/types.py @@ -75,7 +75,6 @@ class ResolvedJudge: config: dict[str, Any] variation_key: str = "" version: int | None = None - is_inverted: bool | None = None @dataclass diff --git a/packages/client/tests/test_evaluations_run.py b/packages/client/tests/test_evaluations_run.py index f1181b6..05ee31a 100644 --- a/packages/client/tests/test_evaluations_run.py +++ b/packages/client/tests/test_evaluations_run.py @@ -812,7 +812,15 @@ async def handler(*args: object) -> dict[str, Any]: @pytest.mark.asyncio -async def test_generation_failed_rows_do_not_fail_the_result() -> None: +async def test_failed_rows_fail_the_result() -> None: + """A row the server scored and marked failed must fail the gate. + + This reverses the previous assertion, which was written when runs were + generation-only -- a row then either generated or errored, and nothing + produced a "failed", so excluding failed_rows was unobservable. With + criteria it is the normal way a run fails, and a gate that ignores it exits + 0 on a run where every row failed its judge. + """ transport = SequencedTransport( [ response(200, {"id": "dataset-id", "name": "golden"}), @@ -856,7 +864,7 @@ async def handler(*args: object) -> dict[str, Any]: ) assert result.summary.failed_rows == 1 - assert result.passed is True + assert result.passed is False @pytest.mark.asyncio @@ -1135,10 +1143,16 @@ async def handler( ) assert result.passed is True + # kind and judgeKey are what let ai-evaluator store this as a judge rather + # than default it to a deepeval metric and reject the key. successDirection + # is deliberately absent: LaunchDarkly injects it from the judge's AI Config + # on the way through, so the SDK must not assert a direction of its own. assert transport.requests[2]["body"]["criteria"] == [ { "criterionType": "$ld:ai:judge:accuracy", - "options": {}, + "kind": "judge", + "judgeKey": "$ld:ai:judge:accuracy", + "options": {"threshold": 0.5}, } ] assert transport.requests[3]["body"] == {"source": "api", "datasetId": "dataset-id"} @@ -1153,30 +1167,37 @@ async def handler( assert judge_event["variationKey"] == "default" assert judge_event["version"] == 12 assert len(judge_event["eventId"]) == 64 - # No threshold was set on the Judge, so there's nothing to compare the score - # against -- verdict must be omitted rather than sent as verdict=None. + # The SDK reports the score and never rules on it: ai-evaluator derives the + # verdict at ingest from the criterion's stored threshold and direction. assert "verdict" not in judge_event @pytest.mark.asyncio @pytest.mark.parametrize( - ("is_inverted", "threshold", "expected_verdict"), + ("is_inverted", "threshold"), [ - # score is fixed at 0.86 in the handler below. - (False, 0.8, "pass"), # upper-is-better, 0.86 >= 0.8 - (False, 0.9, "fail"), # upper-is-better, 0.86 < 0.9 - (True, 0.9, "pass"), # lower-is-better, 0.86 <= 0.9 - (True, 0.5, "fail"), # lower-is-better, 0.86 > 0.5 - (None, 0.8, "pass"), # unresolved direction defaults to upper-is-better + # The score is fixed at 0.86 below. Under every direction and on both + # sides of the threshold, the SDK reports the same thing: a score. + (False, 0.8), + (False, 0.9), + (True, 0.9), + (True, 0.5), + (None, 0.8), ], ) -async def test_run_with_ld_judge_computes_verdict_from_score_threshold_and_direction( +async def test_run_with_ld_judge_never_sends_a_verdict( monkeypatch: pytest.MonkeyPatch, stub_sdk_client: MagicMock, is_inverted: bool | None, threshold: float, - expected_verdict: str, ) -> None: + """Pass/fail is ai-evaluator's ruling, not the SDK's. + + Parametrized over isInverted -- including the served-payload value -- to + pin that the SDK does not compare even when it could: verdict policy has to + be able to change server-side and apply to runs already recorded, which it + cannot if each SDK release freezes its own comparison. + """ transport = SequencedTransport( [ response(200, {"id": "dataset-id", "name": "golden"}), @@ -1255,7 +1276,128 @@ async def handler( assert result.passed is True events = [call.args[2] for call in stub_sdk_client.track.call_args_list] judge_event = next(event for event in events if event.get("kind") == "judge") - assert judge_event["verdict"] == expected_verdict + assert judge_event["score"] == 0.86 + assert "verdict" not in judge_event + assert "successDirection" not in judge_event + + +@pytest.mark.asyncio +async def test_judges_resolve_once_per_run_not_once_per_row( + monkeypatch: pytest.MonkeyPatch, + stub_sdk_client: MagicMock, +) -> None: + """One resolution for the whole run, however many rows it has. + + extract_variation reads flag delivery, an in-memory store that updates + within seconds of a UI edit, so resolving per row would let an edit + mid-run change the rubric text, judge model, and provider between one row + and the next -- rows in a single run scored against different judges. The + online path does resolve per invocation (judges.build_judge_tasks), so + routing the offline runner through it for convenience is a live way to + reintroduce this. + """ + transport = SequencedTransport( + [ + response(200, {"id": "dataset-id", "name": "golden"}), + response( + 200, + dataset_page( + [ + {"rowIndex": 0, "input": "one", "variables": {}}, + {"rowIndex": 1, "input": "two", "variables": {}}, + {"rowIndex": 2, "input": "three", "variables": {}}, + ], + total=3, + ), + ), + response(201, {"id": "evaluation-id", "name": "support-qa"}), + response( + 201, + {"id": "run-id", "evaluationId": "evaluation-id", "state": "PENDING"}, + ), + response( + 200, + {"statusCounts": {"total": 3, "passed": 3, "error": 0, "pending": 0}}, + ), + ] + ) + + resolutions: list[str] = [] + + async def fake_extract_variation( + key: str, context: dict[str, Any] + ) -> dict[str, Any]: + resolutions.append(key) + return { + "config": { + "provider": {"name": "OpenAI"}, + "model": {"name": "gpt-4o"}, + "instructions": "Judge {{response_to_evaluate}}", + }, + "meta": {"variationKey": "default", "version": 12}, + } + + monkeypatch.setattr( + "launchdarkly_ai_server.evaluations.runner.extract_variation", + fake_extract_variation, + ) + evals = init_evaluations(api_token="token", sdk_key="sdk-key", transport=transport) + + async def handler( + config: dict[str, Any], + user_input: str | None, + tool_handlers: dict[str, Callable[..., Any]], + variables: dict[str, Any], + ) -> dict[str, Any]: + if "Judge" in config.get("instructions", ""): + return {"output": '{"score": 0.9, "reasoning": "fine"}'} + return {"output": "generated"} + + await evals.run( + project_key="proj", + key="support-qa", + dataset="golden", + handler=handler, + generation={"provider": "OpenAI", "model": "gpt-4o"}, + criteria=[Judge(key="$ld:ai:judge:accuracy")], + ) + + assert resolutions == ["$ld:ai:judge:accuracy"] + + +def test_scorer_lower_is_better_reaches_the_criteria_wire() -> None: + """A scorer counting something unwanted -- regex hits, edit distance -- + inverts, and only the SDK knows: there is no AI Config for the proxy to + read a scorer's direction off, so what the caller declares is the sole + source ai-evaluator derives its verdict from.""" + + def count_violations(row: DatasetRow, output: Any) -> float: + return 0.0 + + scorer = Scorer( + name="policy-violations", + fn=count_violations, + threshold=0.0, + success_direction="lower_is_better", + ) + + assert scorer.to_criteria_wire() == { + "criterionType": "policy-violations", + "kind": "scorer", + "successDirection": "lower_is_better", + "options": {"threshold": 0.0}, + } + + +def test_judge_threshold_defaults_so_a_criterion_is_always_rulable() -> None: + """A judge with no threshold gives LaunchDarkly nothing to compare against, + so the criterion would be stored and never ruled on.""" + assert Judge(key="$ld:ai:judge:accuracy").to_criteria_wire() == { + "criterionType": "$ld:ai:judge:accuracy", + "kind": "judge", + "judgeKey": "$ld:ai:judge:accuracy", + "options": {"threshold": 0.5}, + } @pytest.mark.asyncio @@ -1350,8 +1492,15 @@ def check_refund(row: DatasetRow, output: Any) -> bool: ) assert result.passed is True + # A scorer has no LaunchDarkly-side config, so unlike a judge it declares + # its own direction and the proxy leaves it alone. assert transport.requests[2]["body"]["criteria"] == [ - {"criterionType": "refund-exists", "options": {"threshold": 1.0}} + { + "criterionType": "refund-exists", + "kind": "scorer", + "successDirection": "higher_is_better", + "options": {"threshold": 1.0}, + } ] assert transport.requests[3]["body"] == {"source": "api", "datasetId": "dataset-id"} events = [call.args[2] for call in stub_sdk_client.track.call_args_list] @@ -1641,7 +1790,7 @@ async def test_failed_evaluation_event_tracking_skips_event_but_completes_run( accuracy_judge_variation(monkeypatch) def track(event_name: str, *args: Any) -> None: - if event_name == "$ld:ai:offline-evals:evaluation": + if event_name == "$ld:ai:offline-evals:criterion": raise RuntimeError("event pipeline unavailable") stub_sdk_client.track = MagicMock(side_effect=track)