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: 31 minutes Limit details: You’ve used the included review currently available. 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 within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day 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 79.50% 79.93% +0.43%
==========================================
Files 272 272
Lines 19019 19045 +26
==========================================
+ Hits 15121 15224 +103
+ Misses 3898 3821 -77 ☔ 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).
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).
02ef5c1 to
8010bd4
Compare
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).
8010bd4 to
9b9d619
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.
9b9d619 to
018a619
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