[bot] Merge master/0e0f3dd3 into rel/dev - #1759
Merged
Merged
Conversation
Per-tool-call wall time (call receipt to result receipt) was invisible -- avg_latency_s is one number for the whole turn, and the reasoning sidecar has section titles but no timing. Stamps call_ts/result_ts on each ToolCallEvent as it streams in and adds build_latency_breakdown() to sum wall time by tool name, so a slow turn (e.g. 117s alert creation) can be attributed to the specific tool call(s) that dominated it. Experimental -- not wired into every agentic evaluator yet, just the plumbing plus visualization (next commit).
…tic) Wires build_latency_breakdown into both the single-shot VisualizationEvaluator and the agentic path's RunResult/AgenticEvalOutcome, so detail.latency_breakdown shows up in eval results for visualization/vis_agentic without any change to the eval harness's runner or result serialization -- detail already flows through verbatim.
The prior version only summed tool-call wall time, leaving most of a slow turn unexplained (e.g. 47s of tool time out of 119s total avg_latency_s). Timestamps each reasoning step as it streams in (ReasoningStepEvent) and merges it into the same timeline as tool calls in build_latency_breakdown: the gap between any two consecutive points -- a tool call starting, a tool call's result landing, or a reasoning step being emitted -- is charged to whichever was "active" during it. This accounts for effectively the whole turn instead of just its tool-call portion. Also fixes multi-turn accumulation in agentic/visualization.py: each turn's timestamps restart near 0, so RunResult now shifts them by a running turn_offset (each turn's own turn_wall_clock_sec) before concatenating -- without it, turn 2's points would overlap turn 1's in the merged timeline.
Reasoning summaries are a full paragraph ("**Title**\n\nlots of detail...");
using the whole thing as a dict key made latency_breakdown unreadable.
Extracts just the bolded title (same convention gdc-mic-ai-evaluation's own
generate_dashboard_summary.py already uses for these reasoning blocks),
falling back to a truncated snippet when a step has no title.
latency_breakdown reasoning labels were fuzzy-matchable to the .reasoning.json sidecar only by title text -- not reliable, since titles can repeat (two distinct steps both titled "Considering data analysis"). Each reasoning step now carries its own 0-based index (same position it occupies in ChatResult.reasoning_steps and the sidecar's ordered list), embedded directly in the latency_breakdown label, e.g. "reasoning:3:Evaluating YoY metrics" -> sidecar block 3. No sidecar format change needed -- the index was always implicit in list position, just not visible from the latency_breakdown side. Also fixes a real single-shot-path bug this surfaced: core/runner.py's _run_one_item took reasoning_steps from whichever run executed LAST, but best_detail (and any latency_breakdown inside it) from whichever run ranked BEST -- for K>1 these can be different runs entirely, so the sidecar and detail.latency_breakdown could each describe a different attempt with no way to tell. Now both come from the same best-ranked run's chat_result. The agentic path (core/agentic/visualization.py) already did this correctly via its own `best` RunResult, so only the single-shot runner needed the fix.
…t-by-name
A dict keyed by "tool:name"/"reasoning:index:title" aggregated repeat calls
of the same tool together and had no way to say what ran before what --
the actual pipeline order was lost. Returns a list of steps instead, each
{"seq", "kind", "name", "index", "duration_s"}: "seq" is the step's real
execution-order position across tools and reasoning combined, "index" is
its position within its own kind's source list (ToolCallEvent.index or
ReasoningStepEvent.index) for looking up the full record -- arguments/result
for a tool call, or the full paragraph in the .reasoning.json sidecar for a
reasoning step. The same tool called twice now produces two separate
entries in their real order, not one summed total.
Also adds ToolCallEvent.index (optional, mirroring the existing
call_ts/result_ts pattern) and re-numbers both tool and reasoning indices
across turns in the agentic visualization path, alongside the existing
turn_offset shift for timestamps.
…cribes avg_latency_s is a mean across all K runs. detail.latency_breakdown (and reasoning_steps/the sidecar) only ever describe the single best-ranked run. For K=1 these coincide, but for K>1 avg_latency_s is not a valid number to check latency_breakdown's coverage against -- it can describe a run whose own latency differs substantially from the mean of all K attempts, with no way to tell by how much. best_run_latency_s is that specific run's own latency, threaded through core/runner.py (same best_chat_result tracking as the earlier reasoning-steps fix) and into the JSON report next to avg_latency_s.
…ators Extends the visualization-only latency_breakdown work to every enabled test_kind: agentic_alert_skill, agentic_metric_skill, agentic_guardrail, agentic_conversation (all multi-turn -- same tool/reasoning index-offset shift across turns as visualization.py), plus the single-shot general_question, guardrail, and search_tool evaluators (single chat_result, no turn accumulation needed). conversation.py is nested two loops deep (logical turns x clarification sub-turns) -- added a conversation-wide tool_call_events/reasoning_step_events accumulator alongside the existing reasoning_steps one, offset-shifted per physical send_message() call regardless of which loop it's in. Test fixtures in test_agentic_conversation.py used bare MagicMock() chat results without call_ts/result_ts/index/reasoning_step_events/ turn_wall_clock_sec set (predating this capture) -- updated them to set these to their real no-op defaults, matching what an actual un-instrumented ChatResult already provides.
…ert)" This reverts commit 1bc4a38.
Pure whitespace/line-wrap, no functional change -- I'd only run `ruff check` locally, not `ruff format --check`, which is CI's actual lint-and-format-check job.
Per-tool-call/reasoning-step latency breakdown (detail.latency_breakdown)
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## rel/dev #1759 +/- ##
===========================================
+ Coverage 80.59% 80.61% +0.02%
===========================================
Files 272 272
Lines 19223 19362 +139
===========================================
+ Hits 15492 15609 +117
- Misses 3731 3753 +22 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
2 participants
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.
🚀 Automated PR to perform merge from master into rel/dev with changes up to 0e0f3dd (created by https://github.com/gooddata/gooddata-python-sdk/actions/runs/32841245757).