feat(gooddata-eval): capture agent reasoning steps in ChatResult - #1708
feat(gooddata-eval): capture agent reasoning steps in ChatResult#1708Tomkess wants to merge 3 commits into
Conversation
|
Warning Review limit reached
Next review available in: 34 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (15)
📝 WalkthroughWalkthroughThe change carries reasoning step summaries from SSE responses through ChangesReasoning Step Reporting
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SSEClient
participant ChatResult
participant AgenticEvaluator
participant AgenticRunner
participant ItemReport
participant JSONReport
SSEClient->>ChatResult: Provide reasoning_steps
ChatResult->>AgenticEvaluator: Supply reasoning_steps
AgenticEvaluator-->>AgenticRunner: Return steps or attach them to an assertion
AgenticRunner->>ItemReport: Store reasoning_steps
ItemReport->>JSONReport: Serialize reasoning
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/gooddata-eval/tests/test_runner.py (1)
261-279: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for multi-run reasoning retention.
Line 120 in
packages/gooddata-eval/src/gooddata_eval/core/runner.pypreserves the previous list when a later run returns[]. Add a test with two runs: the first returns reasoning steps and the second returns an empty list. Assert that the report keeps the first run's steps.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/gooddata-eval/tests/test_runner.py` around lines 261 - 279, Add a multi-run test near the existing reasoning_steps tests, using a backend that returns reasoning steps on the first call and an empty list on the second. Invoke run_items with runs=2 and assert the item report retains the first run’s reasoning steps after the later empty result.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/gooddata-eval/tests/test_runner.py`:
- Around line 261-279: Add a multi-run test near the existing reasoning_steps
tests, using a backend that returns reasoning steps on the first call and an
empty list on the second. Invoke run_items with runs=2 and assert the item
report retains the first run’s reasoning steps after the later empty result.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fa87fd52-43ed-41b4-8aa1-6d92546bc59a
📒 Files selected for processing (7)
packages/gooddata-eval/src/gooddata_eval/core/chat/sse_client.pypackages/gooddata-eval/src/gooddata_eval/core/models.pypackages/gooddata-eval/src/gooddata_eval/core/reporting/json_report.pypackages/gooddata-eval/src/gooddata_eval/core/runner.pypackages/gooddata-eval/tests/test_reporting.pypackages/gooddata-eval/tests/test_runner.pypackages/gooddata-eval/tests/test_sse_client.py
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1708 +/- ##
==========================================
+ Coverage 78.40% 78.85% +0.45%
==========================================
Files 271 271
Lines 18741 18767 +26
==========================================
+ Hits 14693 14798 +105
+ Misses 4048 3969 -79 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Addresses CodeRabbit nitpick on PR #1708: a later run returning no reasoning events must not clobber an earlier run's captured steps (runner.py:120's `or` pattern, same as conversation_id/response_id).
The SSE reasoning events were already being read to produce reasoning_step_count, but the step text itself was discarded. Keep it as reasoning_steps on ChatResult/ItemReport and surface it in the JSON report so eval consumers can inspect the agent's actual reasoning trace, not just how many steps it took.
Addresses CodeRabbit nitpick on PR #1708: a later run returning no reasoning events must not clobber an earlier run's captured steps (runner.py:120's `or` pattern, same as conversation_id/response_id).
413d756 to
02ef5c1
Compare
…path 6001d2f wired ChatResult.reasoning_steps through runner.py's generic single-turn path only. The agentic-CLI path (cli/agentic_runner.py -> evaluate_agentic_*) builds its own ItemReport and never touched it, so agentic_alert_skill/agentic_metric_skill/agentic_conversation items could never produce a reasoning trace, no matter what the platform emitted. Accumulates reasoning_steps across every send_message call in each of the three evaluators' run loops, attaches it to the run/turn result, and surfaces it from evaluate_agentic_* either as the return value (pass) or as an attribute on the raised exception (fail) -- mirroring the existing conversation_id-on-exception idiom in ChatClient.ask(). run_agentic_items picks it up from either path onto ItemReport.reasoning_steps, which json_report.py already serializes unconditionally. general_question/guardrail/search_tool/visualization are left untouched -- their evaluate_agentic_* functions still return None, unchanged.
02ef5c1 to
8010bd4
Compare
1 participant
Summary
gooddata-eval's SSE chat client already reads the assistant'sreasoningevents (content.type == "reasoning",content.summaryper step) to computereasoningStepCount— but discarded the step text itself. This PR keeps it.ChatResultgainsreasoning_steps: list[str](aliasreasoningSteps), alongside the existingreasoning_step_count.ItemReport(runner.py) carriesreasoning_stepsfrom each run'sChatResult, same pattern asconversation_id/response_id(last non-empty value wins acrosspass_at_kruns)."reasoning"key, so it flows throughgd-eval run --jsonunchanged.No new cost: the SSE stream already carries this data for every reasoning-capable request: this only stops throwing away.
Test plan
test_sse_client.py(reasoning text retained, empty when no reasoning events)test_runner.py(reasoning_stepscarried ontoItemReport, empty when absent)test_reporting.py("reasoning"key present/empty inbuild_json_reportoutput)pytest -q→ 254 passed, no regressionsruff checkclean on all changed filesSummary by CodeRabbit
New Features
Bug Fixes
Tests