feat(gooddata-eval): capture conversation_id/response_id through the agentic-CLI path - #1713
Conversation
|
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 |
9b9d619 to
018a619
Compare
1c328c4 to
b47e42c
Compare
| model_version_override: str | None, | ||
| reasoning_effort: ReasoningEffort | None = None, | ||
| ) -> list[str] | None: | ||
| ) -> tuple[list[str], str, str | None] | list[str] | None: |
There was a problem hiding this comment.
Create a dedicated dataclass for this output please
There was a problem hiding this comment.
Fixed in dcf9f5b — added AgenticEvalOutcome (reasoning_steps, conversation_id, response_id) to core/models.py and made evaluate_agentic_alert_skill/metric_skill/conversation return it instead of the bare tuple. _dispatch_agentic's return type is now AgenticEvalOutcome | list[str] | None, and the isinstance(outcome, tuple) unpack in run_agentic_items is now isinstance(outcome, AgenticEvalOutcome). Updated the tests that unpacked the old tuple.
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>
dcf9f5b to
8e7d622
Compare
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