Skip to content

gooddata-eval: drop the dead skills fallback spelling in set_skills parsing #1780

Description

@Tomkess

Split out of #1762 discussion (agreed there as separable).

_set_skills_declarations() in packages/gooddata-eval/src/gooddata_eval/core/agentic/conversation.py hedges across two argument spellings:

names = args.get("skill_names")
if names is None:
    names = args.get("skills")

skill_names is the real key: the set_skills tool declares skill_names: list[str] and its run() reads that field. Nothing emits or reads a bare skills. Verified against the gen-ai service while answering the replace-vs-append question on #1762.

So the fallback is provably dead. It was also actively harmful once: test_agentic_conversation.py's _skills_tc helper emitted {"skills": [...]}, and every conversation test passed through the fallback, exercising a payload the platform never sends. Fixed in #1762 (9965722e), but the fallback is what let it go unnoticed — a hedge that silently accepts a wrong shape hides test bugs rather than tolerating real traffic.

Scope:

  • Remove the or args.get("skills") branch.
  • Grep for remaining "skills"-as-argument-key spellings in tests. test_visualization_evaluator.py and test_agentic_visualization.py already use skill_names and are correct.

Worth doing after (or together with) the result-vs-arguments change, since that touches the same parse.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions