fix(gooddata-eval): read set_skills' result rather than its arguments to determine active skills

[cobanfurkanx]

Closes #1779

Description

_set_skills_declarations() / _final_skill_declaration() in packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py and _check_visualization_skill_activated in core/evaluators/visualization.py previously derived the active skill set solely from the set_skills tool call's arguments (what the agent requested).

However, what the agent requests is not always what actually becomes active:

• The service drops requested names it does not recognise.
• The service pulls in skills declared as dependencies of the ones requested.

The tool call's own result echoes back the authoritative post-replacement active set (e.g. skills_to_activate). Reading the result is strictly truer to what actually became active than re-deriving it from the request arguments.

Changes

• Implemented _extract_active_skills(tc) to prefer the authoritative post-replacement set in tc.parsed_result() (skills_to_activate, skill_names, skills), falling back to tc.parsed_arguments().get("skill_names") when the result is absent, unparseable, or errored.
• Updated _set_skills_declarations() in conversation.py to use _extract_active_skills().
• Updated _check_visualization_skill_activated() in visualization.py to use _extract_active_skills().
• Added unit tests in both test_agentic_conversation.py and test_visualization_evaluator.py covering:
  1. Unrecognised name requested (dropped by result -> not credited).
  2. Implicit dependency pulled in by result (not in arguments -> credited).
  3. Result missing (fallback path).
  4. Result unparseable (fallback path).
  5. Result errored (fallback path).
  6. Explicit empty result (deactivates skills).

Summary by CodeRabbit

• Bug Fixes
  • Skill activation and evaluation now reflect the skills reported by a successful tool result, including dependencies and explicit empty results.
  • When results are missing, unreadable, or indicate failure, evaluation falls back to the requested skills.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: cfc95777-c865-445d-89d0-3e74dd4ce263

---

📥 Commits

Reviewing files that changed from the base of the PR and between 8168136 and dc12ddc.

---

📒 Files selected for processing (4)

• packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py
• packages/gooddata-eval/src/gooddata_eval/core/evaluators/visualization.py
• packages/gooddata-eval/tests/test_agentic_conversation.py
• packages/gooddata-eval/tests/test_visualization_evaluator.py

---

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

---

📝 Walkthrough

Walkthrough

The conversation evaluator and visualization evaluator now derive active skills from set_skills results when usable. Both fall back to requested skill arguments when results are missing, unparseable, or indicate an error. Tests cover dropped requested skills, included dependencies, and fallback cases.

Changes

Active Skill Evaluation

Layer / File(s)	Summary
Conversation skill declarations and coverage packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py, packages/gooddata-eval/tests/test_agentic_conversation.py	Conversation declarations use skills from the tool result when available, including an explicit empty list. Tests cover argument fallback and how result skills affect routing and coverage.
Visualization activation checks packages/gooddata-eval/src/gooddata_eval/core/evaluators/visualization.py, packages/gooddata-eval/tests/test_visualization_evaluator.py	The visualization evaluator checks result skills before falling back to requested skills. Tests cover included and excluded skills, missing or unparseable results, and error results.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Low

---

Merge Risk: ⚪ Minimal · up to dc12d

No actionable issue remains from this review; the change is mergeable after normal checks.

Pre-merge checks | 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and accurately summarizes the main change: active skills now come from the set_skills result instead of only from the requested arguments.
Linked Issues check		The changes satisfy the coding requirements in #1779. Both conversation skill declarations and visualization activation now read the parsed set_skills result first. They support skills_to_activate, sk…
Out of Scope Changes check		The pull request changes only the two active-skill detection paths and their unit tests. The test changes directly verify the behavior requested by #1779. No unrelated source behavior or unrelated fil…
Docstring Coverage		Docstring coverage is 88.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 4 files.

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{A rabbit checks the skill list twice,
The result sets the names precise.
If results fail or go away,
The request can guide the way.
Soft paws cheer as tests agree.}

_{Comment @coderabbitai help to get the list of available commands.}