Conversation
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 "/ <threshold>" 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.
Self-review found the regression test for the dialog crash did not exercise the code it named. It passed a metric with requiresThreshold false, which the constructor's filter dropped, leaving metricsInfo empty; the template then rendered the hardcoded fallback and the slider holding the optional chaining was never created. The test passed with the guard reverted. The filter also tested `!!metricValueInfo?.interval`, which made the guard unreachable by construction: nothing without an interval could survive to reach it. Narrow the filter to requiresThreshold, its actual job, and let a metric that needs a threshold but arrives without an interval through. The slider now falls back to 0..1 rather than binding undefined bounds, and the form validates only that a value is set, since there are no bounds to check against. Replace the vacuous test with two that assert the metric is offered, that the slider reaches the page, and how the control validates. Add the coverage chat.component.ts was missing for metricHasVerdict and getMetricColor, following the rubricPassed tests alongside them. Also record why the informational metrics are dropped rather than shown: collectMetrics always emits a threshold, which those metrics reject, and the always-on section the server's metrics-info filter is waiting on still has to be built.
i-yliu
marked this pull request as ready for review
September 15, 2026 22:23
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.
The eval framework is gaining a metric kind that reports a value and never
passes or fails (
EvalStatus.INFORMATIONAL, value4). The efficiencymetrics — tool call count, inference call count, token usage — are reported on
every run and carry it. The Dev UI does not know that status, and its eval
views assume every metric either passed or failed, so such a metric is
presented as a failure:
numerator, so a case whose only quality metric passed reads
1/4, not1/1.PASSEDwith the error colourand border, so a token count renders exactly like a failed assertion.
/ <threshold>unconditionally, leaving a dangling slashfor a metric that has none, and the run-summary row leaves a dangling colon.
Teach the UI the status and render it neutrally: a metric with no verdict gets
no verdict styling, no threshold suffix, and stays out of the ratio.
Separately, stop the run-configuration dialog from dereferencing
metricValueInfo.intervalunconditionally. The interval is optional on thewire, and a metric without one took the whole threshold form down with
Uncaught TypeError: Cannot read properties of undefined (reading 'maxValue').The dialog now offers only the metrics it can actually configure — those that
require a threshold and carry an interval to bound the slider by. Metrics
without one are always on and are reported without the user selecting them.
MetricValueInfo.intervalis now optional andMetricsInfocarriesrequiresThreshold, matching what the server already sends.Testing
Served one eval history containing
INFORMATIONALresults through bothbundles, with the same backend on each side so the frontend is the only
variable.
Before
After
The ratio goes from
1/4to1/1, the three efficiency chips lose the failurecolour and border, and the dangling
1.00 /disappears.Also checked the ratio across four histories, to cover both tally paths:
1/41/11/51/20/30/02/82/2The dialog fix was verified separately, by letting metrics without an interval
actually reach the form. Before, "Run All" renders an empty dialog and the
console shows the
maxValueTypeError above; after, the form renders andsimply does not offer those metrics.
New unit tests cover the ratio in
eval-tab.component.spec.tsand the dialog'sfiltering in
run-eval-config-dialog.component.spec.ts, including a regressiontest that the form renders rather than throwing on a metric with no interval.