Skip to content

Commit 7bc56fa

Browse files
Jan Brandejsclaude
andcommitted
fix(gooddata-eval): count timed-out turns and fix HTML change labels
Review of the latency report found two places where the response-time population and the scored runs disagree, three where the HTML report disagrees with the console or labels the wrong run, and one repeated computation. A turn that ended in a ChatError (a turn or item deadline, an error event, an incomplete turn) was never recorded as a trace, although evaluators score its partial result as a run. The slowest turns went missing from the per-response percentiles. send_message now records the turn's wall clock from the partial result before re-raising. An item that raised outside its evaluator's own failure reporting kept the turns it had completed but reported no runs, so it showed up in the per-response table and not in the per-conversation one. Its turns are now dropped (TraceRecorder.discard), along with the runs it never reported. The pass-rate card divided by every test case, skipped ones included, while the console divides by those evaluated, so 10 passed of 14 with 2 skipped read 71% against 83%. The card, and the recomputed figures of a filtered view, now use one passRateOf helper. The page's pass_rate figure was read by nothing and is removed; the JSON field is unchanged. The Since previous run labels described earlier runs when they should describe the latest: a test case skipped in the latest run was labelled Regressed beside a SKIP cell, and one absent from the previous run but present before it was labelled New instead of Fixed or Flaky. A skipped latest run now has no label, and New and Removed look at every earlier run. EvalReport.by_mode is a cached_property, since the console, the comparison and both JSON builders each rebuilt and re-sorted it. The README describes the changed definitions. The HTML rules are tested by running the page's own functions in Node, skipped where Node is absent. jira: trivial risk: low Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
1 parent 1054889 commit 7bc56fa

11 files changed

Lines changed: 205 additions & 27 deletions

File tree

‎packages/gooddata-eval/README.md‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,9 @@ Each item also states its `mode`, its `trace_latencies_s` and its `run_latencies
340340
Its latency is that turn's response time, from sending the message to the end of its stream
341341
(`turn_wall_clock_sec`), recorded where the chat client completes the turn. It holds none of the LLM judge,
342342
the simulated user or conversation create/delete. `dashboard_summary` is timed the same way around its one
343-
request. A turn that raised was never completed and has no entry.
343+
request. A turn cut off by its turn or item deadline (or ended by an error event) still counts, with the
344+
time it ran for: those are the slowest turns, and the run that scored them is in the per-run figures too. An
345+
item that errored outside its evaluator's own failure reporting contributes neither.
344346
- **Single-shot** runs are exactly one trace, so `traces_s` is also the latency of a run.
345347
- **Multi-turn** runs span several traces, so they get both views: `traces_s` over every turn of every
346348
conversation (comparable with single-shot), and `runs_s`, the wall clock of each whole conversation from just
@@ -409,8 +411,8 @@ simulated user), which are `single_shot` and `multi_turn` in the JSON.
409411
the runs in play has it (so "Failed" over previous and latest finds every test case that broke in either), and the
410412
expression is evaluated once per run in play with the same any-run rule. A filtered page states how many test cases
411413
it covers, and each tier has its own Reset.
412-
- **Results** — three headline cards for the latest run: test cases passed, the average quality score and the median
413-
time per attempt. With an earlier run each card carries one change line in words, "down from 86% on Sep 28", rather
414+
- **Results** — three headline cards for the latest run: test cases passed (of those evaluated: skipped ones are not
415+
counted, as in the console's percentage), the average quality score and the median time per attempt. With an earlier run each card carries one change line in words, "down from 86% on Sep 28", rather
414416
than a signed number. The line is green when that direction is good and red when it is not, with a chevron and
415417
screen-reader text (*Better* / *Worse*) so colour is not the only signal. From three runs each card also has a
416418
sparkline of every run: hover a point for its value, click it to compare the latest run with that one instead of the
@@ -428,8 +430,9 @@ simulated user), which are `single_shot` and `multi_turn` in the JSON.
428430
decimal and no redundant ".0" (`3s`, `4.6s`).
429431
- **A test case table**, newest first: the latest run's result, a **Since previous run** column, then the earlier runs
430432
going back in time (click any result to open the drawer on that run). The change column reads *Regressed* (passed in
431-
the previous run, not in the latest), *Fixed*, *Flaky* (flipped between pass and fail more than once), *New* or
432-
*Removed*. With several runs the table opens on the regressions. Two time columns
433+
the previous run, not in the latest), *Fixed*, *Flaky* (flipped between pass and fail more than once), *New*
434+
(in no earlier run) or *Removed* (in an earlier run, gone from the latest). A test case skipped in the latest run
435+
has no label, since the change would describe earlier runs. With several runs the table opens on the regressions. Two time columns
433436
keep the populations apart: **Response time** is the median of the test case's assistant responses, and **Time per
434437
attempt** is the mean wall-clock of a whole attempt (the responses plus the judge and, for conversations, the
435438
simulated user). With more than two runs shown they follow the latest run only. Hover a value to see every sample

‎packages/gooddata-eval/src/gooddata_eval/cli/agentic_runner.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -545,6 +545,12 @@ def _process_item(index: int, item: DatasetItem) -> ItemReport:
545545
# judge broke should not also report the agent as costing 0s. Kinds that
546546
# attach no timings to the exception keep their 0.0 defaults.
547547
_apply_timings(item_report, getattr(exc, "timings", None))
548+
# Phase timings stay, but the per-turn response times do not: this item reports no
549+
# runs (and so no run latencies), and keeping its turns would put them in the
550+
# response population with no attempt beside them. An item that completed 2 of 3
551+
# runs and then raised would otherwise show up in the per-response table and not in
552+
# the per-conversation one.
553+
traces.discard()
548554
finally:
549555
item_report.latency_s = time.perf_counter() - t0
550556
item_report.trace_latencies_s = traces.stop()

‎packages/gooddata-eval/src/gooddata_eval/core/chat/sse_client.py‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -588,10 +588,21 @@ def _do() -> ChatResult:
588588
result.turn_wall_clock_sec = time.monotonic() - t0
589589
return result
590590

591-
result = _retry_transient(_do, is_retryable=_is_retryable_exc)
592-
# One completed turn is one gen-ai trace. Recorded here, from the figure the turn already
593-
# carries, so every caller is covered and no extra clock is read. A turn that raised is
594-
# not recorded: it has no completed response to time.
591+
try:
592+
result = _retry_transient(_do, is_retryable=_is_retryable_exc)
593+
except ChatError as exc:
594+
# A turn that ended in a ChatError (a turn or item deadline, an error event, an
595+
# incomplete turn) is still one gen-ai trace, and several evaluators score its
596+
# partial result as a run. Recording it keeps the response population matching the
597+
# runs that were scored: left out, the slowest turns -- the ones that hit a deadline --
598+
# would vanish and pull the tail down exactly when the agent is slowest.
599+
if exc.partial_result is not None:
600+
note_trace_latency(exc.partial_result.turn_wall_clock_sec)
601+
raise
602+
# One turn is one gen-ai trace. Recorded here, from the figure the turn already carries,
603+
# so every caller is covered and no extra clock is read. A turn that failed without a
604+
# ChatError carries no partial result, so there is no response to time and it is not
605+
# recorded.
595606
note_trace_latency(result.turn_wall_clock_sec)
596607
return result
597608

‎packages/gooddata-eval/src/gooddata_eval/core/reporting/report_template.html‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -415,13 +415,15 @@ <h2>Test cases</h2>
415415
const total = c.total ?? s.total ?? 0, passed = c.passed ?? s.passed ?? 0;
416416
return {
417417
total, passed, errored: s.errored ?? 0, skipped: s.skipped ?? 0,
418-
pass_rate: c.pass_rate ?? (total ? passed / total : 0),
419418
avg_quality_score: c.avg_quality_score,
420419
avg_latency_s: c.avg_latency_s ?? s.avg_latency_s,
421420
total_latency_s: c.total_latency_s ?? s.latency_s,
422421
by_mode: c.by_mode || s.by_mode || null,
423422
};
424423
}
424+
// Share of the evaluated test cases that passed, for reported and recomputed figures alike. A skipped test case never
425+
// ran, so it is left out of the divisor, as the console's percentage does; null when nothing was evaluated.
426+
const passRateOf = (f) => (f.total - f.skipped > 0 ? f.passed / (f.total - f.skipped) : null);
425427
const r3 = (v) => Math.round(v * 1000) / 1000;
426428
// Linear interpolation between closest ranks, as gd-eval computes it, so P50 is the median.
427429
function percentile(sorted, p) {
@@ -459,7 +461,7 @@ <h2>Test cases</h2>
459461
}
460462
return {
461463
total: items.length, passed, errored: items.filter((it) => it.error != null).length,
462-
skipped: items.filter((it) => it.skipped).length, pass_rate: items.length ? passed / items.length : 0,
464+
skipped: items.filter((it) => it.skipped).length,
463465
avg_quality_score: evaluated.length ? mean(evaluated.map(qualityOf)) : 0,
464466
avg_latency_s: runs ? latency / runs : 0, total_latency_s: latency, by_mode,
465467
};
@@ -521,7 +523,7 @@ <h2>Test cases</h2>
521523
function renderSummary() {
522524
const base = runsShown.length > 1 ? compareBase() : null;
523525
$('#summary').innerHTML = '<h2>Results</h2><div class="trendgrid">' +
524-
kpiCard({ title: 'Test cases passed', get: (l) => (figures[l].total ? figures[l].passed / figures[l].total : null), fmt: pct, higher: true,
526+
kpiCard({ title: 'Test cases passed', get: (l) => passRateOf(figures[l]), fmt: pct, higher: true,
525527
isSame: (d) => Math.round(d * 100) === 0, base, after: ' <a href="#items-section" class="which">which?</a>' }) +
526528
kpiCard({ title: 'Average quality score', get: (l) => (figures[l].total ? figures[l].avg_quality_score : null), fmt: pct, higher: true,
527529
isSame: (d) => Math.abs(d) < 0.01, base }) +
@@ -843,14 +845,20 @@ <h2>Test cases</h2>
843845
return (va < vb ? -1 : va > vb ? 1 : 0) * sortDir;
844846
});
845847
}
846-
// How a test case moved between the last two runs shown, and whether it has flipped back and forth across all of them.
848+
// How a test case moved between the last two runs it was evaluated in, and whether it has flipped back and forth across
849+
// all of them. Every label describes the latest run, so none is given when that run skipped the test case or lacks it.
847850
const CHANGE_ORDER = { Regressed: 0, New: 1, Removed: 2, Flaky: 3, 'Still failing': 4, Fixed: 5, 'Still passing': 6, '': 7 };
848851
function changeOf(r, runList) {
852+
const cur = r.per[runList[runList.length - 1]];
853+
// New and Removed look at every earlier run, not just the one before: a test case that skipped a run and came back
854+
// is not new, and its pass or fail history still says whether it was fixed or is flaky.
855+
const earlier = runList.slice(0, -1).some((l) => r.per[l]);
849856
if (runList.length > 1) {
850-
const prevIt = r.per[runList[runList.length - 2]], curIt = r.per[runList[runList.length - 1]];
851-
if (curIt && !prevIt) return 'New';
852-
if (prevIt && !curIt) return 'Removed';
857+
if (cur && !earlier) return 'New';
858+
if (!cur && earlier) return 'Removed';
853859
}
860+
// Skipped in the latest run (or missing from it): the earlier runs alone would label a result the row does not show.
861+
if (['none', 'skip'].includes(statusOf(cur))) return '';
854862
const st = runList.map((l) => statusOf(r.per[l])).filter((x) => x !== 'none' && x !== 'skip');
855863
if (st.length < 2) return '';
856864
const ok = st.map((x) => x === 'pass');
@@ -926,7 +934,7 @@ <h2>Test cases</h2>
926934
{ k: 'id', html: 'Test case' }, { k: 'kind', html: 'Category' }, { k: 'mode', html: 'Format' }, { k: 'question', html: 'Question' },
927935
// Newest first: the latest result, what changed since the previous run, then earlier runs going back in time.
928936
...(multi ? [{ k: 's:' + runList[runList.length - 1], html: esc(roleOf(runList[runList.length - 1])) + LATEST_PILL }] : []),
929-
...(multi ? [{ k: 'chg', html: 'Since previous run', help: 'Regressed: passed in the previous run, not in the latest. Fixed: the reverse. Flaky: has flipped between pass and fail more than once across the runs shown. New / Removed: the test case was added to or dropped from the suite.' }] : []),
937+
...(multi ? [{ k: 'chg', html: 'Since previous run', help: 'Regressed: passed in the previous run, not in the latest. Fixed: the reverse. Flaky: has flipped between pass and fail more than once across the runs shown. New / Removed: the test case first appears in the latest run, or was in an earlier run and is gone from the latest. A test case skipped in the latest run has no label.' }] : []),
930938
...(multi ? runList.slice(0, -1).reverse() : runList).map((l) => ({ k: 's:' + l, html: esc(roleOf(l)) })),
931939
...timeRuns.map((l) => ({ k: 'lt:' + l, html: 'Response time' + runHdr(l), help: TRACE_HELP, num: true })),
932940
...timeRuns.map((l) => ({ k: 'lr:' + l, html: 'Time per attempt' + runHdr(l), help: RUN_HELP, num: true })),

‎packages/gooddata-eval/src/gooddata_eval/core/runner.py‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import traceback
66
from concurrent.futures import ThreadPoolExecutor, as_completed
77
from dataclasses import dataclass, field
8-
from functools import partial
8+
from functools import cached_property, partial
99
from typing import Callable, Protocol
1010

1111
from gooddata_eval.core.config import DEFAULT_GATE, EvalGate
@@ -67,8 +67,10 @@ class ItemReport:
6767
# The response time of every chat turn the item completed, in order -- one gen-ai trace
6868
# each, timed from sending the message to the end of its stream. For a single-shot item that
6969
# is one value per run; a multi-turn run contributes one per turn. Unlike `run_latencies_s`
70-
# it holds none of the judge, simulated-user or conversation set-up time. A turn that raised
71-
# has no entry, but a turn whose scoring raised afterwards does: it completed.
70+
# it holds none of the judge, simulated-user or conversation set-up time. A turn that ended in
71+
# a ChatError carrying a partial result has an entry, because evaluators score that result as a
72+
# run; so does a turn whose scoring raised afterwards. An item that errored outside the
73+
# evaluator's own failure reporting has none: it reports no runs for them to sit beside.
7274
trace_latencies_s: list[float] = field(default_factory=list)
7375
# Per-phase breakdown of what the item's time was spent on. Additive to latency_s,
7476
# which remains the item's own critical path. langfuse_latency_s is
@@ -200,10 +202,14 @@ def avg_quality_score(self) -> float:
200202
evaluated = [i.quality_score for i in self.items if not i.skipped and i.error is None]
201203
return sum(evaluated) / len(evaluated) if evaluated else 0.0
202204

203-
@property
205+
@cached_property
204206
def by_mode(self) -> dict[str, dict]:
205207
"""How the run did on its single-shot and on its multi-turn items, with latency for each.
206208
209+
Computed on first read and kept: the console, the comparison and both JSON builders all
210+
read it, and each read would otherwise sort every latency sample again. Read it once the
211+
report is complete -- items added afterwards are not reflected.
212+
207213
``total`` and ``passed`` count the evaluated items of that mode (skipped ones belong to
208214
neither). ``traces_s`` is the distribution of per-trace chat-turn response times; a
209215
single-shot run is one trace, so for that mode it is also the per-run latency. A

‎packages/gooddata-eval/src/gooddata_eval/core/timing.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,11 @@ def stop(self) -> list[float]:
167167
self._token = None
168168
return self.latencies
169169

170+
def discard(self) -> None:
171+
"""Detach and drop what was recorded, for an item whose turns have no scored run to sit beside."""
172+
self.stop()
173+
self.latencies.clear()
174+
170175

171176
def note_trace_latency(seconds: float | None) -> None:
172177
"""Record one completed chat turn on the active recorder, if there is one.

‎packages/gooddata-eval/tests/test_agentic_runner.py‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,14 +1065,33 @@ def test_turns_completed_before_a_failed_gate_still_count_as_traces():
10651065
assert report.items[0].trace_latencies_s == [3.0, 4.0]
10661066

10671067

1068-
def test_turns_completed_before_an_item_errored_still_count_as_traces():
1069-
"""The agent answered those turns however the item ended, so they are real response times."""
1068+
def test_an_item_that_errored_outside_its_evaluator_reports_no_trace_latencies():
1069+
"""Such an item reports no runs, so keeping its turns would put them in the per-response
1070+
population with nothing beside them in the per-conversation one."""
10701071
with patch(
10711072
"gooddata_eval.cli.agentic_runner.evaluate_agentic_alert_skill",
1072-
side_effect=_noting(2.0, then=RuntimeError("simulated user failed")),
1073+
side_effect=_noting(2.0, 3.0, then=RuntimeError("simulated user failed")),
10731074
):
10741075
report = run_agentic_items([_item()], host="http://host", token="tok", workspace_id="ws1", run_ts="2026-01-01")
1075-
assert report.items[0].error is not None
1076+
item = report.items[0]
1077+
assert item.error is not None
1078+
assert item.runs == 0
1079+
assert item.run_latencies_s == []
1080+
assert item.trace_latencies_s == []
1081+
assert report.by_mode["multi_turn"]["traces_s"] is None
1082+
1083+
1084+
def test_an_ungraded_item_keeps_the_turns_it_drove():
1085+
"""Unlike the case above, a judge fault leaves every run's time on the exception, so the
1086+
per-response and per-conversation populations still cover the same item."""
1087+
err = JudgeResponseError("judge returned no readable verdict for any of the 2 run(s)")
1088+
err.runs_effective = 2
1089+
err.run_latencies_s = [7.0, 9.5]
1090+
with patch("gooddata_eval.cli.agentic_runner.evaluate_agentic_alert_skill", side_effect=_noting(2.0, then=err)):
1091+
report = run_agentic_items(
1092+
[_item()], host="http://host", token="tok", workspace_id="ws1", run_ts="2026-01-01", k=2
1093+
)
1094+
assert report.items[0].run_latencies_s == [7.0, 9.5]
10761095
assert report.items[0].trace_latencies_s == [2.0]
10771096

10781097

0 commit comments

Comments
 (0)