feat(gooddata-eval): capture conversation_id/response_id through the agentic-CLI path - #1713
Open
Tomkess wants to merge 1 commit into
Open
Conversation
…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>
|
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 |
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
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
feat/chat-client-reasoning-steps, not master.reasoning_steps, applied toconversation_id/response_id:cli/agentic_runner.py -> evaluate_agentic_*never surfaced them ontoItemReportfor the three agentic kinds (agentic_alert_skill/agentic_metric_skill/agentic_conversation), even though eachChatResultalready carries both.evaluate_agentic_*return fromlist[str]to(reasoning_steps, conversation_id, response_id); attaches all three to the raised exception on failure (mirrors the existingconversation_id-on-exception idiom inChatClient.ask());run_agentic_itemsunpacks either form.response_idis new at theRunResultlayer for all three kinds — captured as the last non-null value across a run's turns, same accumulation pattern already used forreasoning_steps.general_question/guardrail/search_tool/visualizationuntouched — already populated via the single-turnrunner.pypath, unaffected by this gap.Test plan
uv run pytest tests/test_agentic_alert_skill.py tests/test_agentic_conversation.py tests/test_agentic_metric_skill.py tests/test_agentic_runner.py tests/test_alert_skill_evaluator.py tests/test_metric_skill_evaluator.py— 65 passeduv run pytest) — same 9 pre-existing failures with or without this patch (missingopenaiin local env), zero new failuresruff checkclean on all touched filesty check— same 4 pre-existingopenai-unresolved-import diagnostics with or without this patch, zero new diagnostics🤖 Generated with Claude Code