[bot] Merge master/d5e0fa12 into rel/dev - #1734
Merged
Merged
Conversation
…xt classification _is_asking_kda_clarification tried to classify agent responses as "asking for clarification" vs "a final answer" via a "?"-based heuristic, so a simulated user reply was only sent when it matched. That heuristic missed a real, common response shape: a clarifying question immediately followed by a bullet list of the options being offered (e.g. "Which metric?\n- metric A\n- metric B") -- the message doesn't end on "?" itself, so the run gave up after turn 1 instead of ever nudging the simulated user to pick one, scoring a genuinely-ambiguous case as triggered=False. Found via a real CI trace (QA-28800, gpt56luna_openai / globalmart): the chatbot asked to disambiguate between two "Total Net Revenue" metrics -- one of which was the expected answer -- but kda_disambiguated stayed False and the session never got a second turn, confirming the simulated-reply path was never reached. First pass patched the heuristic (wider bullet-char support, "?" no longer needing to be the literal last character, a period_hint gap, a "None 'None'" prompt bug). Review (chi My) pointed out the cost of the two error directions is asymmetric: missing a genuine clarifying question hard-fails the run, while misreading a final answer as one only costs one harmless extra turn (the loop already breaks for good once create_args is set, so a false positive here can never turn a pass into a fail). Every other skill in this package (visualization.py, alert_skill.py) already solves this the cheap way: never classify the text at all, just break on the goal signal (tool called / artifact produced) or an empty response, and otherwise always retry. Patching the KDA-specific heuristic for one more response shape (this round it was "**Option 1**: ..." -- bold markdown with no space after the marker) would have meant chasing an open-ended list of shapes forever. Fix: dropped _is_asking_kda_clarification and _LIST_ITEM_RE entirely. _run_once now matches visualization.py/alert_skill.py's own break conditions -- create_args set, or an empty response -- and otherwise always sends a simulated reply, regardless of what the agent's text says or how it's formatted. _DEFAULT_MAX_ITERATIONS bumped 3 -> 4 (chi My's point: 3 was sized exactly for 2 real questions with zero slack for a wasted turn; every other skill in the package budgets 4-7). Also fixed along the way: - generate_simulated_kda_response only ever knew about measure candidates, even when the agent's question was about the PERIOD to compare instead -- it had nothing period-specific to answer with. Extracted into _build_period_hint(), built from whichever of expected_output's Date Attribute/Analyzed Period/Reference Period fields are present (not requiring all three). - The prompt asserted "an acceptable metric/fact is None 'None'" as a real option when measure_candidates was None/empty (e.g. a period-only question) -- likely to make gpt-4o-mini invent a metric literally named "None". Extracted into _build_clarification_prompt(), which now omits the "For reference, ..." clause entirely when there's nothing usable to reference. Tests: _is_asking_kda_clarification's own unit tests removed along with the function; the end-to-end run_agentic_kda_skill regression tests for the real captured trace and the period-clarification case stay (now exercising the always-retry path instead of a classifier match), plus a new test for the bold- markdown case chi My's review flagged, direct unit tests for _build_period_hint and _build_clarification_prompt, and a bumped _DEFAULT_MAX_ITERATIONS. 48 tests in test_agentic_kda_skill.py, all passing; package suite unchanged at 9 pre-existing unrelated failures (missing openai module in this venv). JIRA: QA-28800
…ristic fix(gooddata-eval): retry KDA on any non-triggering response, drop text classification
|
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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## rel/dev #1734 +/- ##
===========================================
+ Coverage 79.42% 79.44% +0.02%
===========================================
Files 272 272
Lines 18997 19012 +15
===========================================
+ Hits 15088 15105 +17
+ Misses 3909 3907 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
🚀 Automated PR to perform merge from master into rel/dev with changes up to d5e0fa1 (created by https://github.com/gooddata/gooddata-python-sdk/actions/runs/31669773261).