Commit 8f83a9e
committed
feat(gooddata-eval): add KDA-skill agentic evaluator
Adds kda_skill.py to gooddata-eval, evaluating the chatbot's
create_key_driver_analysis/execute_key_driver_analysis tool calls against the
agent_kda_skill Langfuse dataset.
Scope is strictly completion, not field correctness:
strict_pass = triggered AND executed AND success AND turn_completed
Per-field checks (Measure/Date Attribute/Periods/Filters/Summary matching
expected values) are deferred entirely to a follow-up ticket (QA-28699)
rather than half-computed here as scores nothing reads yet.
Two things can extend a run past a single turn, each bounded to
max_iterations: the agent asks a clarifying question (a simulated user reply,
gpt-4o-mini, nudges it forward), or it calls create but not execute in the
same turn (create/execute are tracked independently across turns, so a plain
continuation nudge gives it another turn instead of scoring it as if execute
never happened).
Latency is measured directly by the harness (ChatClient times each
send_message() call around the SSE stream), not re-derived from a Langfuse
trace after the fact. Only the turn that actually completes KDA counts
toward it -- not an earlier disambiguation/continuation turn, and not the
simulated-reply's own OpenAI call. Logged as the kda_turn_wall_clock_sec
Langfuse score; combo_report.py (gdc-nas) reads it directly.
Fixes from review:
- kda_ prefix on pass_at_k/pass_power_k Langfuse scores -- unprefixed,
"pass_at_2" at k=2 collides with visualization.py's own score name, which
gdc-nas's combo_report.py.verdict() checks first when classifying a trace.
- send_message errors no longer propagate out of run_agentic_kda_skill
uncaught -- they now surface as a normal failed run, so it still gets
scored to Langfuse instead of only showing up as a bare JUnit failure.
- ChatError/TransientChatError now carry partial_result, so tool calls that
already succeeded before a later, unrelated error (e.g. a failed final
summary) aren't discarded and misreported as "KDA never triggered".
- turn_completed resets to False in the exception branch, so a crash on a
later iteration can't leave a stale True from an earlier one.
- run_agentic_kda_skill rejects k < 1 -- a bad env-driven KDA_RUN_K value
(0, a typo, negative) previously ran silently once instead of surfacing
the bad config.
- stream_ended is now set the moment the response_ended event line is
parsed, not its data line -- an event with no data payload previously
left the flag unset.
- t0 is set before opening the SSE stream, not after -- it was missing the
connection/server-setup time a caller actually waits through.
- Disambiguated-turn latency no longer double-counts every turn's time plus
the simulated-reply's own OpenAI call.
- The 6 per-field informational correctness scores and their support code
are removed -- team confirmed this PR's scope is trigger+complete only.
- KdaEvaluation.kda_triggered renamed to triggered, matching the other
three core fields (none of which carry the kda_ prefix). The Langfuse
score name kda_triggered is unchanged.
- generate_simulated_kda_response's OpenAI call now has a 30s timeout.
- kda_skill wired into the CLI's agentic dispatcher (agentic_runner.py)
and AGENTIC_TEST_KINDS, matching every other agentic skill, so it can
be run/debugged standalone via gd-eval run instead of only through
gdc-nas's tavern-e2e harness.
- dataset_name default renamed from agent_kda_skill to kda_skill,
matching the no-prefix pattern every other skill uses; no functional
change since gdc-nas always passes it explicitly.
- A few comments/docstrings that had drifted from the multi-turn behavior
corrected: KdaRunResult's docstring ("one message" -> up to
max_iterations), the turn_wall_clock_sec field comment, and
_extract_kda_calls' docstring (pairing is only guaranteed within one
turn; merging across turns is the caller's job).
- expected_output.get("Measure") now guards for non-dict shapes --
DatasetItem.expected_output on the gdc-nas side allows str/list, not
just dict, and a list-shaped item previously raised AttributeError,
silently swallowed by the broad except and disabling disambiguation
with only a WARNING.
- _DEFAULT_MAX_ITERATIONS raised from 2 to 3 -- 2 was lower than every
other agentic skill (visualization=4, alert_skill=6, metric_skill=7)
and left no room for a case that needs both disambiguation and a
create/execute turn split.
- turn_wall_clock_sec's field comment corrected again -- it accumulates
from the first create call through every turn attempted after it,
whether or not execute ever completes, not only "through the turn
that completed execute".
- kda_pass_at_k/kda_pass_power_k no longer logged to Langfuse -- matches
metric_skill/alert_skill/guardrail/search_tool/general_question, which
all compute the pair but never log it at their default k=1; nothing
reads a kda_pass_at_1 score today, and the score name shifts if k ever
changes, silently splitting any Langfuse view built on the old name.
- sse_client.py's t0 comment corrected again -- being per-attempt
excludes not just the sleep backoff between retries, but the entire
duration of any earlier failed attempt too.
- Two missing test cases added: create succeeds but execute never
arrives even after the continuation nudge (previously only tested
running out of iterations via repeated clarification, not via nudge);
and a run combining both extension paths (disambiguation, then a
create-without-execute continuation) exhausting its budget safely.
- KdaRunResult.eval renamed to evaluation -- it shadowed the eval
builtin and is part of the published surface (exported in
core/agentic/__init__.py's __all__); fixed now, before gdc-nas starts
consuming this module.
- turn_wall_clock_sec reverted to counting only the turn that actually
completed KDA (both create and execute resolved), matching this PR's
original, already-reviewed latency definition. An earlier change in
this PR summed a create-only turn's time into it on the theory that a
continuation turn (create succeeds, execute lands a turn later) is
real gen-ai processing time -- that theory doesn't match the agreed
definition: create being tracked across turns is only what lets the
harness recognize completion when execute arrives late, it was never
meant to change what latency measures.
- The "continuation" mechanism itself (create/execute tracked
independently across turns, with a "Please proceed." nudge if create
succeeded without execute in the same turn) is removed entirely, not
just its latency accounting. Read gdc-nas's actual KDA skill (system
prompt + orchestration loop): create and execute are always called
together in one turn, no confirmation step -- this scenario doesn't
happen. The only real no-execute case is the org having data-sharing
with the LLM off, which removes execute_key_driver_analysis from the
tool list entirely (permanent gap, not a delayed call); no chat nudge
can work around that, so there was nothing for this mechanism to
correctly handle in the first place.
- _DEFAULT_MAX_ITERATIONS raised from 2 to 3 for a different reason than
the removed continuation mechanism: metric and analyzed-period
ambiguity can each need their own clarifying question, so a case
ambiguous on both can legitimately take 2 rounds before create is
ever called (confirmed against real test runs). Still asking after
that is the model failing to resolve the object, not KDA itself.
JIRA: QA-288001 parent 9103989 commit 8f83a9e
8 files changed
Lines changed: 1578 additions & 7 deletions
File tree
- packages/gooddata-eval
- src/gooddata_eval
- cli
- core
- agentic
- chat
- tests
Lines changed: 12 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
11 | 11 | | |
12 | 12 | | |
13 | 13 | | |
| 14 | + | |
14 | 15 | | |
15 | 16 | | |
16 | 17 | | |
| |||
38 | 39 | | |
39 | 40 | | |
40 | 41 | | |
| 42 | + | |
41 | 43 | | |
42 | 44 | | |
43 | 45 | | |
| |||
159 | 161 | | |
160 | 162 | | |
161 | 163 | | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
162 | 174 | | |
163 | 175 | | |
164 | 176 | | |
| |||
Lines changed: 14 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
33 | 41 | | |
34 | 42 | | |
35 | 43 | | |
| |||
56 | 64 | | |
57 | 65 | | |
58 | 66 | | |
| 67 | + | |
59 | 68 | | |
60 | 69 | | |
61 | 70 | | |
| |||
69 | 78 | | |
70 | 79 | | |
71 | 80 | | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
72 | 84 | | |
73 | 85 | | |
74 | 86 | | |
| |||
81 | 93 | | |
82 | 94 | | |
83 | 95 | | |
| 96 | + | |
84 | 97 | | |
85 | 98 | | |
86 | 99 | | |
87 | 100 | | |
88 | 101 | | |
89 | 102 | | |
90 | 103 | | |
| 104 | + | |
91 | 105 | | |
92 | 106 | | |
93 | 107 | | |
| |||
0 commit comments