feat(gooddata-eval): capture agent reasoning steps in ChatResult - #1708
Merged
Conversation
|
Warning Review limit reached
Next review available in: 48 seconds 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?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. 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.69% 80.14% +0.45%
==========================================
Files 272 272
Lines 19024 19101 +77
==========================================
+ Hits 15161 15309 +148
+ Misses 3863 3792 -71 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Tomkess
added a commit
that referenced
this pull request
Aug 4, 2026
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).
3 tasks
Tomkess
added a commit
that referenced
this pull request
Aug 5, 2026
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).
Tomkess
force-pushed
the
feat/chat-client-reasoning-steps
branch
2 times, most recently
from
August 5, 2026 08:01
02ef5c1 to
8010bd4
Compare
4 tasks
Tomkess
added a commit
that referenced
this pull request
Aug 19, 2026
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).
Tomkess
force-pushed
the
feat/chat-client-reasoning-steps
branch
2 times, most recently
from
August 19, 2026 14:29
9b9d619 to
018a619
Compare
hkad98
previously approved these changes
Aug 20, 2026
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).
…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.
Tomkess
force-pushed
the
feat/chat-client-reasoning-steps
branch
from
August 20, 2026 12:02
018a619 to
7d4496f
Compare
…agentic-CLI path 8010bd4 wired reasoning_steps through cli/agentic_runner.py -> evaluate_agentic_*, but conversation_id/response_id stayed unset on ItemReport for every agentic kind (agentic_alert_skill/agentic_metric_skill/agentic_conversation) -- each ChatResult already carries both, and conversation_id was already threaded up to the Alert/Metric/ConversationRunResult layer, but neither ever reached the top-level evaluate_agentic_* return value or its failure exception, so run_agentic_items had nothing to read. Mirrors the reasoning_steps idiom exactly: widens each evaluate_agentic_*'s return from list[str] to (reasoning_steps, conversation_id, response_id), attaches all three to the raised exception on failure, and has run_agentic_items unpack either form (tuple or the untouched kinds' bare list/None) onto ItemReport.conversation_id /response_id. response_id is new at the RunResult layer for all three kinds -- captured as the last non-null value across a run's turns, same pattern already used for reasoning_steps accumulation. general_question/guardrail/search_tool/visualization untouched (already populated via the single-turn runner.py path, not this one). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… dataclass evaluate_agentic_alert_skill/metric_skill/conversation returned a bare tuple[list[str], str, str | None], and _dispatch_agentic's return type was a tuple | list[str] | None union unpacked via isinstance(outcome, tuple). Introduced AgenticEvalOutcome (reasoning_steps, conversation_id, response_id) in core/models.py and used it as the return type instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…onse-ids feat(gooddata-eval): capture conversation_id/response_id through the agentic-CLI path
hkad98
approved these changes
Aug 20, 2026
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.
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