feat(gooddata-eval): capture agent reasoning steps in ChatResult - #1708
feat(gooddata-eval): capture agent reasoning steps in ChatResult#1708Tomkess wants to merge 2 commits into
Conversation
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.
|
Warning Review limit reached
Next review available in: 50 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 (1)
📝 WalkthroughWalkthroughThe change carries reasoning step summaries from SSE responses through ChangesReasoning Step Reporting
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant SSEClient
participant ChatResult
participant Runner
participant JSONReport
SSEClient->>ChatResult: Collect reasoning steps
ChatResult->>Runner: Return reasoning_steps
Runner->>JSONReport: Provide item reasoning_steps
JSONReport-->>Runner: Serialize reasoning
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1708 +/- ##
=======================================
Coverage 78.30% 78.30%
=======================================
Files 271 271
Lines 18689 18692 +3
=======================================
+ Hits 14634 14637 +3
Misses 4055 4055 ☔ 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).
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