From 53b54124ac5f8a9593030690ea87b8450c988614 Mon Sep 17 00:00:00 2001 From: Yi Liu Date: Tue, 8 Sep 2026 20:51:15 +0000 Subject: [PATCH] fix(eval): render metrics that report a value but reach no verdict The eval framework is gaining a metric kind that reports a value and never passes or fails (EvalStatus.INFORMATIONAL). The Dev UI assumed every metric either passed or failed, so such a metric was presented as a failure: counted in the "passed / evaluated" denominator but never the numerator, coloured with the error colour and border, and given a dangling "/ " suffix it has no threshold for. Render it neutrally instead: no verdict styling, no threshold suffix, and out of the ratio. Also stop the run-configuration dialog from dereferencing metricValueInfo.interval unconditionally. The interval is optional on the wire and a metric without one took the whole threshold form down. The dialog now offers only the metrics it can configure. --- src/app/components/chat/chat.component.html | 28 ++++++---- src/app/components/chat/chat.component.ts | 20 ++++++- .../eval-tab/eval-tab.component.html | 7 ++- .../eval-tab/eval-tab.component.spec.ts | 51 +++++++++++++++++ .../components/eval-tab/eval-tab.component.ts | 10 +++- .../run-eval-config-dialog.component.html | 2 +- .../run-eval-config-dialog.component.spec.ts | 55 +++++++++++++++++++ .../run-eval-config-dialog.component.ts | 27 ++++++--- src/app/core/models/Eval.ts | 11 +++- 9 files changed, 185 insertions(+), 26 deletions(-) diff --git a/src/app/components/chat/chat.component.html b/src/app/components/chat/chat.component.html index 1caa841b..dc171551 100644 --- a/src/app/components/chat/chat.component.html +++ b/src/app/components/chat/chat.component.html @@ -724,17 +724,19 @@
@for (metric of result.overallEvalMetricResults; track metric.metricName) {
{{ metric.metricName | formatMetricName }}
- {{ metric.score != null ? (metric.score | number:'1.2-2') : '?' }} - - / {{ metric.threshold | number:'1.2-2' }} - + @if (metric.threshold != null) { + + / {{ metric.threshold | number:'1.2-2' }} + + }
@@ -743,14 +745,16 @@
{{ metric.metricName }}
Actual: {{ metric.score != null ? + [style.color]="getMetricColor(metric)">{{ metric.score != null ? (metric.score | number:'1.2-2') : '?' }}
-
Threshold: {{ - metric.threshold | number:'1.2-2' }}
-
Min: {{ - getMetricMin(metric.metricName) }}
-
Max: {{ - getMetricMax(metric.metricName) }}
+ @if (metricHasVerdict(metric)) { +
Threshold: {{ + metric.threshold | number:'1.2-2' }}
+
Min: {{ + getMetricMin(metric.metricName) }}
+
Max: {{ + getMetricMax(metric.metricName) }}
+ }
@let desc = getMetricDescription(metric.metricName); @if (desc) { diff --git a/src/app/components/chat/chat.component.ts b/src/app/components/chat/chat.component.ts index 7b19e8cb..8ccb90bf 100644 --- a/src/app/components/chat/chat.component.ts +++ b/src/app/components/chat/chat.component.ts @@ -42,7 +42,7 @@ import { catchError, distinctUntilChanged, filter, first, map, shareReplay, star import { URLUtil } from '../../../utils/url-util'; import { AgentRunRequest } from '../../core/models/AgentRunRequest'; -import { EvalCase, EvaluationResult } from '../../core/models/Eval'; +import { EvalCase, EvaluationResult, EvalStatus } from '../../core/models/Eval'; import { Session, SessionState } from '../../core/models/Session'; import { Event as AdkEvent, Part } from '../../core/models/types'; import { UiEvent } from '../../core/models/UiEvent'; @@ -512,6 +512,24 @@ export class ChatComponent implements OnInit, AfterViewInit, OnDestroy { return info?.description || ''; } + /** + * Whether a metric reached a pass/fail verdict. An informational metric + * reports a value and never does, so it must not be styled as a failure + * merely for not having passed. + */ + metricHasVerdict(metric: any): boolean { + return metric?.evalStatus !== EvalStatus.INFORMATIONAL; + } + + /** Green when passed, red when failed, neutral when there is no verdict. */ + getMetricColor(metric: any): string { + if (!this.metricHasVerdict(metric)) { + return 'var(--mat-sys-on-surface-variant)'; + } + return metric?.evalStatus === EvalStatus.PASSED ? '#2e7d32' : + 'var(--mat-sys-error)'; + } + /** Whether a single rubric result passed, per the backend verdict/score. */ rubricPassed(rubric: any): boolean { if (rubric?.verdict !== undefined && rubric?.verdict !== null) { diff --git a/src/app/components/eval-tab/eval-tab.component.html b/src/app/components/eval-tab/eval-tab.component.html index c666e0f3..794ac9fc 100644 --- a/src/app/components/eval-tab/eval-tab.component.html +++ b/src/app/components/eval-tab/eval-tab.component.html @@ -297,8 +297,11 @@

Past Runs

@for (evalMetric of currentHistoryMetrics(); track evalMetric) { - {{ evalMetric.metricName | formatMetricName }}: - {{ evalMetric.threshold | number:'1.2-2' }} + + {{ evalMetric.metricName | formatMetricName }}@if (evalMetric.threshold != null) {: + {{ evalMetric.threshold | number:'1.2-2' }}} }
diff --git a/src/app/components/eval-tab/eval-tab.component.spec.ts b/src/app/components/eval-tab/eval-tab.component.spec.ts index f4a421a6..685a338c 100644 --- a/src/app/components/eval-tab/eval-tab.component.spec.ts +++ b/src/app/components/eval-tab/eval-tab.component.spec.ts @@ -331,4 +331,55 @@ describe('EvalTabComponent', () => { expect(evalService.getEvalSet).toHaveBeenCalledWith('my-app', 'my-set'); }); }); + + describe('getMetricsScore', () => { + // EvalStatus: 1 PASSED, 2 FAILED, 3 NOT_EVALUATED, 4 INFORMATIONAL. + it('counts only metrics that reach a verdict', () => { + const evalRes = { + evalMetricResults: [ + {metricName: 'tool_trajectory_avg_score', evalStatus: 1}, + {metricName: 'response_match_score', evalStatus: 2}, + {metricName: 'safety_v1', evalStatus: 3}, + ], + }; + + expect((component as any).getMetricsScore(evalRes)).toBe('1/2'); + }); + + it('excludes informational metrics from the ratio', () => { + // The efficiency metrics are reported on every run and never pass or + // fail. Counting them would show a fully passing case as 1/4. + const evalRes = { + evalMetricResults: [ + {metricName: 'tool_trajectory_avg_score', evalStatus: 1}, + {metricName: 'tool_call_count_v1', evalStatus: 4}, + {metricName: 'inference_call_count_v1', evalStatus: 4}, + {metricName: 'token_usage_v1', evalStatus: 4}, + ], + }; + + expect((component as any).getMetricsScore(evalRes)).toBe('1/1'); + }); + + it('excludes informational metrics tallied per invocation', () => { + const evalRes = { + evalMetricResultPerInvocation: [ + { + evalMetricResults: [ + {metricName: 'tool_trajectory_avg_score', evalStatus: 1}, + {metricName: 'token_usage_v1', evalStatus: 4}, + ], + }, + { + evalMetricResults: [ + {metricName: 'tool_trajectory_avg_score', evalStatus: 2}, + {metricName: 'token_usage_v1', evalStatus: 4}, + ], + }, + ], + }; + + expect((component as any).getMetricsScore(evalRes)).toBe('1/2'); + }); + }); }); diff --git a/src/app/components/eval-tab/eval-tab.component.ts b/src/app/components/eval-tab/eval-tab.component.ts index 280b4c82..99b95020 100644 --- a/src/app/components/eval-tab/eval-tab.component.ts +++ b/src/app/components/eval-tab/eval-tab.component.ts @@ -579,10 +579,16 @@ export class EvalTabComponent implements OnInit, OnChanges { let passed = 0; let total = 0; - // Excludes NOT_EVALUATED so the ratio reads "passed / evaluated". + // Counts only metrics that reach a verdict, so the ratio reads + // "passed / judged". NOT_EVALUATED produced nothing to judge, and + // INFORMATIONAL reports a value without ever passing or failing; counting + // either one would make a fully passing case read as a partial one. const tally = (results: any[]) => { for (const r of results) { - if (r.evalStatus === EvalStatus.NOT_EVALUATED) continue; + if (r.evalStatus === EvalStatus.NOT_EVALUATED || + r.evalStatus === EvalStatus.INFORMATIONAL) { + continue; + } total += 1; if (r.evalStatus === EvalStatus.PASSED) passed += 1; } diff --git a/src/app/components/eval-tab/run-eval-config-dialog/run-eval-config-dialog.component.html b/src/app/components/eval-tab/run-eval-config-dialog/run-eval-config-dialog.component.html index a379d5ad..d17c76d0 100644 --- a/src/app/components/eval-tab/run-eval-config-dialog/run-eval-config-dialog.component.html +++ b/src/app/components/eval-tab/run-eval-config-dialog/run-eval-config-dialog.component.html @@ -97,7 +97,7 @@

Run evaluation

Threshold
- + diff --git a/src/app/components/eval-tab/run-eval-config-dialog/run-eval-config-dialog.component.spec.ts b/src/app/components/eval-tab/run-eval-config-dialog/run-eval-config-dialog.component.spec.ts index f8aeac6d..78ccabd9 100644 --- a/src/app/components/eval-tab/run-eval-config-dialog/run-eval-config-dialog.component.spec.ts +++ b/src/app/components/eval-tab/run-eval-config-dialog/run-eval-config-dialog.component.spec.ts @@ -176,6 +176,61 @@ describe('RunEvalConfigDialogComponent', () => { }); }); + describe('with metrics that need no threshold', () => { + // An informational metric reports a value and never passes or fails, so + // the backend sends it with requiresThreshold false and no value interval. + const INFORMATIONAL_METRIC = { + metricName: 'token_usage_v1', + description: 'Tokens consumed', + metricValueInfo: {}, + requiresThreshold: false, + }; + + it('does not offer them for selection', async () => { + const {component} = await createComponent({ + evalMetrics: [], + metricsInfo: [...METRICS_INFO, INFORMATIONAL_METRIC], + }); + + expect(component.metricsInfo.map((m) => m.metricName)).toEqual([ + 'tool_trajectory_avg_score', + 'response_match_score', + ]); + expect(component.evalForm.get('token_usage_v1_selected')).toBeNull(); + expect(component.evalForm.get('token_usage_v1_threshold')).toBeNull(); + }); + + it('never emits them, so the backend is not sent a threshold it rejects', + async () => { + const {component, dialogRef} = await createComponent({ + evalMetrics: [], + metricsInfo: [...METRICS_INFO, INFORMATIONAL_METRIC], + }); + component.evalForm.get('tool_trajectory_avg_score_selected') + ?.setValue(true); + + component.onStart(); + + const arg = dialogRef.close.calls.mostRecent().args[0] as any; + expect(arg.metrics.map((m: any) => m.metricName)).toEqual([ + 'tool_trajectory_avg_score', + ]); + }); + + it('renders the form rather than failing on the missing interval', + async () => { + // Regression test: the threshold slider used to read + // `metricValueInfo.interval.minValue` unconditionally, so a metric + // without an interval took the whole dialog down. + const {fixture} = await createComponent({ + evalMetrics: [], + metricsInfo: [INFORMATIONAL_METRIC], + }); + + expect(() => fixture.detectChanges()).not.toThrow(); + }); + }); + describe('without metricsInfo (fallback)', () => { let component: RunEvalConfigDialogComponent; let dialogRef: jasmine.SpyObj>; diff --git a/src/app/components/eval-tab/run-eval-config-dialog/run-eval-config-dialog.component.ts b/src/app/components/eval-tab/run-eval-config-dialog/run-eval-config-dialog.component.ts index 52fd34ce..bca31770 100644 --- a/src/app/components/eval-tab/run-eval-config-dialog/run-eval-config-dialog.component.ts +++ b/src/app/components/eval-tab/run-eval-config-dialog/run-eval-config-dialog.component.ts @@ -112,7 +112,15 @@ export class RunEvalConfigDialogComponent { private fb: FormBuilder, @Inject(MAT_DIALOG_DATA) public data: EvalConfigData) { this.evalMetrics = this.data.evalMetrics || []; - this.metricsInfo = this.data.metricsInfo || []; + // This dialog asks the user to select metrics and set a threshold for + // each, with the slider bounded by the metric's value interval. A metric + // that needs no threshold has nothing to configure here, and carries no + // interval to bound a slider by, so it is not offered. Those metrics are + // always on and are reported without the user selecting them. + this.metricsInfo = (this.data.metricsInfo || []) + .filter( + (metric) => metric.requiresThreshold !== false && + !!metric.metricValueInfo?.interval); this.runForm = this.fb.group({ runMode: [DEFAULT_RUN_MODE], @@ -133,11 +141,13 @@ export class RunEvalConfigDialogComponent { this.evalForm.addControl(`${metric.metricName}_selected`, this.fb.control(isSelected)); const interval = metric.metricValueInfo.interval; - this.evalForm.addControl(`${metric.metricName}_threshold`, this.fb.control(threshold, [ - Validators.required, - Validators.min(interval.minValue), - Validators.max(interval.maxValue) - ])); + const validators = [Validators.required]; + if (interval) { + validators.push( + Validators.min(interval.minValue), Validators.max(interval.maxValue)); + } + this.evalForm.addControl( + `${metric.metricName}_threshold`, this.fb.control(threshold, validators)); }); // Fallback if metricsInfo is empty, add the hardcoded ones to avoid empty UI if backend fails @@ -169,7 +179,10 @@ export class RunEvalConfigDialogComponent { private getDefaultThreshold(metric: MetricsInfo): number { if (metric.metricName === 'tool_trajectory_avg_score') return 1.0; if (metric.metricName === 'response_match_score') return 0.7; - return metric.metricValueInfo.interval.maxValue; + // Default to the top of the metric's range, which for a score metric means + // "must be perfect". Metrics without a range are not offered by this + // dialog, so the fallback only guards against a malformed MetricInfo. + return metric.metricValueInfo.interval?.maxValue ?? 1.0; } onReset(): void { diff --git a/src/app/core/models/Eval.ts b/src/app/core/models/Eval.ts index ad948b4a..4e59d340 100644 --- a/src/app/core/models/Eval.ts +++ b/src/app/core/models/Eval.ts @@ -37,7 +37,9 @@ export const DEFAULT_EVAL_METRICS: EvalMetric[] = [ ]; export declare interface MetricValueInfo { - interval: { + // Absent for metrics whose value is unbounded, such as the informational + // efficiency metrics (token and call counts). + interval?: { minValue: number; openAtMin: boolean; maxValue: number; @@ -49,6 +51,9 @@ export declare interface MetricsInfo { metricName: string; description: string; metricValueInfo: MetricValueInfo; + // False for metrics that report a value without passing or failing. Such a + // metric has no threshold to configure and no interval to bound one by. + requiresThreshold?: boolean; } export declare interface Invocation { @@ -91,6 +96,10 @@ export enum EvalStatus { PASSED = 1, FAILED = 2, NOT_EVALUATED = 3, + // The metric reported a value but reached no verdict, so it is neither a + // pass nor a failure. Distinct from NOT_EVALUATED, which means the metric + // produced nothing at all. + INFORMATIONAL = 4, } export declare interface EvaluationResult {