From 3a661a8bd669fa450534f1e695883459b3035032 Mon Sep 17 00:00:00 2001 From: jana-selva Date: Tue, 29 Sep 2026 13:28:22 +0530 Subject: [PATCH 01/11] Implement packaged skill support --- doc/changes/unreleased.md | 5 +- .../features/agent_skills/index.rst | 19 +- exasol/toolbox/nox/_skills.py | 12 +- exasol/toolbox/nox/tasks.py | 4 +- .../references/nox-sessions.md | 4 +- exasol/toolbox/util/skills.py | 2 +- test/integration/skills_integration_test.py | 28 +++ test/unit/nox/_skills_test.py | 16 +- test/unit/skills_test.py | 170 ++++++++++++++---- test/unit/util/skill_utils_test.py | 2 +- 10 files changed, 206 insertions(+), 56 deletions(-) create mode 100644 test/integration/skills_integration_test.py diff --git a/doc/changes/unreleased.md b/doc/changes/unreleased.md index f8092cb65..8bdb0e172 100644 --- a/doc/changes/unreleased.md +++ b/doc/changes/unreleased.md @@ -3,6 +3,7 @@ ## Features * #940: Added shared validation for packaged agent skills and the `skills:check` Nox session. -* #938: Added the `skills:install` Nox session for installing the packaged PTB agent skill. +* #938: Added the `skills:install` Nox session for installing packaged agent skills. +* #963: Extended packaged skill checks and installation to support multiple skills. * #942: Added api-contract-audit skill for identifying mismatches between type annotations, docstrings, and runtime - behavior \ No newline at end of file + behavior diff --git a/doc/user_guide/features/agent_skills/index.rst b/doc/user_guide/features/agent_skills/index.rst index 9e4f8141f..7b9d5980c 100644 --- a/doc/user_guide/features/agent_skills/index.rst +++ b/doc/user_guide/features/agent_skills/index.rst @@ -18,19 +18,20 @@ unfinished TODO markers or forbidden repository-specific metadata, and has no duplicated Markdown lines. Nox command examples are kept in the skill's ``references/nox-sessions.md`` file. -These shared checks are intentionally separate from skill-specific tests. When -adding a skill, add its expected files and behavior assertions to that skill's -own test module, while ``skills:check`` covers the rules common to all skills. +These shared checks are intentionally separate from skill-specific tests. The +test suite validates the structure of every packaged skill and validates the +structure of every available ``eval_cases.yml``. When adding a skill, add its +expected files and behavior assertions to the shared skill test patterns. -Installing the PTB skill ------------------------- +Installing packaged skills +--------------------------- -Projects can install the PTB skill packaged by their current PTB dependency with: +Projects can install all skills packaged by their current PTB dependency with: .. code-block:: shell poetry run -- nox -s skills:install -The session copies the packaged skill into -``.agents/skills/exasol-python-toolbox``. Existing files in that skill directory -are replaced so the installed copy stays aligned with the PTB version. +The session copies each packaged skill into its own directory below +``.agents/skills``. Existing files in those skill directories are replaced so +the installed copies stay aligned with the PTB version. diff --git a/exasol/toolbox/nox/_skills.py b/exasol/toolbox/nox/_skills.py index 2d968bf07..8367f5bbc 100644 --- a/exasol/toolbox/nox/_skills.py +++ b/exasol/toolbox/nox/_skills.py @@ -22,6 +22,7 @@ def _format_skill_errors(skill_name: str, errors: tuple[str, ...]) -> str: def check_skills(session: Session) -> None: """Validate the common structure and content rules for packaged skills.""" failures = {} + # Discover skills at runtime so newly packaged skills are installed too. for skill_name in get_packaged_skill_names(): errors = validate_skill(skill_name) if errors: @@ -35,9 +36,12 @@ def check_skills(session: Session) -> None: @nox.session(name="skills:install", python=False) -def install_ptb_skill(session: Session) -> None: - """Install the PTB skill into the project's local agent skill directory.""" +def install_skills(session: Session) -> None: + """Install all packaged skills into the project's local agent directory.""" from noxconfig import PROJECT_CONFIG - target = install_skill(target_directory=PROJECT_CONFIG.agent_skills_path) - session.log(f"Installed {target.name} skill to {target}") + for skill_name in get_packaged_skill_names(): + target = install_skill( + skill_name, target_directory=PROJECT_CONFIG.agent_skills_path + ) + session.log(f"Installed {target.name} skill to {target}") diff --git a/exasol/toolbox/nox/tasks.py b/exasol/toolbox/nox/tasks.py index 3793036ea..c298f85fd 100644 --- a/exasol/toolbox/nox/tasks.py +++ b/exasol/toolbox/nox/tasks.py @@ -10,7 +10,7 @@ "integration_tests", "lint", "check_skills", - "install_ptb_skill", + "install_skills", "open_docs", "prepare_release", "type_check", @@ -61,7 +61,7 @@ def check(session: Session) -> None: updated, ) from exasol.toolbox.nox._release import prepare_release -from exasol.toolbox.nox._skills import check_skills, install_ptb_skill +from exasol.toolbox.nox._skills import check_skills, install_skills from exasol.toolbox.nox._shared import ( Mode, _integration_test_context, diff --git a/exasol/toolbox/skills/exasol-python-toolbox/references/nox-sessions.md b/exasol/toolbox/skills/exasol-python-toolbox/references/nox-sessions.md index 82761c107..063b45d39 100644 --- a/exasol/toolbox/skills/exasol-python-toolbox/references/nox-sessions.md +++ b/exasol/toolbox/skills/exasol-python-toolbox/references/nox-sessions.md @@ -23,8 +23,8 @@ The sessions below match the PTB version that includes this skill. | Session | Use | Notes | | --- | --- | --- | -| `skills:check` | Validate packaged PTB skills. | It checks common structure and content rules. | -| `skills:install` | Install the PTB agent skill. | It updates `.agents/skills/exasol-python-toolbox` from the installed PTB package. | +| `skills:check` | Validate packaged skills. | It checks common structure and content rules for every packaged skill. | +| `skills:install` | Install packaged agent skills. | It updates each packaged skill below `.agents/skills` from the installed PTB package. | ## Test sessions diff --git a/exasol/toolbox/util/skills.py b/exasol/toolbox/util/skills.py index fcf9e9fd0..6eae1ff0d 100644 --- a/exasol/toolbox/util/skills.py +++ b/exasol/toolbox/util/skills.py @@ -61,7 +61,7 @@ def get_packaged_skill_names() -> tuple[str, ...]: ) except (FileNotFoundError, ModuleNotFoundError) as error: raise RuntimeError( - "Packaged PTB skills are unavailable. Reinstall exasol-toolbox " + "Packaged skills are unavailable. Reinstall exasol-toolbox " "with its package resources." ) from error diff --git a/test/integration/skills_integration_test.py b/test/integration/skills_integration_test.py new file mode 100644 index 000000000..4ce440198 --- /dev/null +++ b/test/integration/skills_integration_test.py @@ -0,0 +1,28 @@ +from unittest.mock import Mock + +import noxconfig + +from exasol.toolbox.nox import _skills +from exasol.toolbox.util.skills import get_packaged_skill_names + + +def test_skills_check_validates_all_packaged_skills(): + # Use real package resources here; unit tests separately cover failure + # formatting with mocked skill names and validation results. + packaged_skills = get_packaged_skill_names() + + assert {"api-contract-audit", "exasol-python-toolbox"}.issubset(packaged_skills) + + session = Mock() + _skills.check_skills(session) + + session.error.assert_not_called() + + +def test_skills_install_installs_all_packaged_skills(tmp_path, monkeypatch): + monkeypatch.setattr(noxconfig, "PROJECT_CONFIG", Mock(agent_skills_path=tmp_path)) + + _skills.install_skills(Mock()) + + for skill_name in get_packaged_skill_names(): + assert (tmp_path / skill_name / "SKILL.md").is_file() diff --git a/test/unit/nox/_skills_test.py b/test/unit/nox/_skills_test.py index 052ef5344..f18149f3e 100644 --- a/test/unit/nox/_skills_test.py +++ b/test/unit/nox/_skills_test.py @@ -39,19 +39,25 @@ def test_check_skills_reports_all_failures(monkeypatch, nox_session): assert "two:\n - missing SKILL.md" in message -def test_install_ptb_skill_uses_project_skill_directory( +def test_install_skills_uses_project_skill_directory( monkeypatch, nox_session, tmp_path ): target_directory = tmp_path / ".agents" / "skills" - target = target_directory / "exasol-python-toolbox" + targets = {name: target_directory / name for name in ("one", "two")} monkeypatch.setattr( noxconfig, "PROJECT_CONFIG", Mock(agent_skills_path=target_directory), ) - install = Mock(return_value=target) + monkeypatch.setattr( + _skills, "get_packaged_skill_names", Mock(return_value=("one", "two")) + ) + install = Mock(side_effect=lambda name, target_directory: targets[name]) monkeypatch.setattr(_skills, "install_skill", install) - _skills.install_ptb_skill(nox_session) + _skills.install_skills(nox_session) - install.assert_called_once_with(target_directory=target_directory) + assert install.call_args_list == [ + (("one",), {"target_directory": target_directory}), + (("two",), {"target_directory": target_directory}), + ] diff --git a/test/unit/skills_test.py b/test/unit/skills_test.py index 373ecbcfd..d0f0d433c 100644 --- a/test/unit/skills_test.py +++ b/test/unit/skills_test.py @@ -1,15 +1,17 @@ from pathlib import Path from subprocess import run from zipfile import ZipFile +from collections.abc import Mapping +import pytest from ruamel.yaml import YAML from exasol.toolbox.util.skills import ( PTB_SKILL_NAME, get_skill_files, get_skill_path, + get_packaged_skill_names, install_skill, - validate_skill, ) PROJECT_ROOT = Path(__file__).parents[2] @@ -21,18 +23,71 @@ "references/nox-sessions.md", "references/source-routing.md", ] -EVAL_CASES = ( - PROJECT_ROOT - / "test" - / "resources" - / "skills" - / "exasol-python-toolbox" - / "eval_cases.yml" -) -def _load_eval_cases() -> dict: - return YAML(typ="safe").load(EVAL_CASES) +def _eval_cases_path(skill_name: str) -> Path: + return ( + PROJECT_ROOT / "test" / "resources" / "skills" / skill_name / "eval_cases.yml" + ) + + +def _load_eval_cases(skill_name: str) -> dict: + return YAML(typ="safe").load(_eval_cases_path(skill_name)) + + +def _skills_with_eval_cases() -> list[str]: + # Keep this data-driven so adding a packaged skill requires no test edit. + return [ + skill_name + for skill_name in get_packaged_skill_names() + if _eval_cases_path(skill_name).is_file() + ] + + +def _validate_eval_cases(eval_cases: object, skill_name: str) -> list[str]: + # Eval cases are test resources, so validate their reusable schema here + # instead of coupling production skill discovery to test-only files. + errors: list[str] = [] + if not isinstance(eval_cases, Mapping): + return ["evaluation cases must be a mapping"] + if eval_cases.get("version") != 1: + errors.append("version must be 1") + if eval_cases.get("skill") != skill_name: + errors.append(f"skill must be {skill_name}") + + cases = eval_cases.get("cases") + if not isinstance(cases, list) or not cases: + return errors + ["cases must be a non-empty list"] + + ids: list[str] = [] + for index, case in enumerate(cases): + if not isinstance(case, Mapping): + errors.append(f"case {index} must be a mapping") + continue + case_id = case.get("id") + if not isinstance(case_id, str) or not case_id.strip(): + errors.append(f"case {index} must have a non-empty id") + else: + ids.append(case_id) + for field in ("category", "prompt"): + value = case.get(field) + if not isinstance(value, str) or not value.strip(): + errors.append(f"case {index} must have a non-empty {field}") + + expected = case.get("expected") + if not isinstance(expected, Mapping): + errors.append(f"case {index} expected must be a mapping") + continue + for field in ("must_include", "must_not_include"): + values = expected.get(field) + if not isinstance(values, list) or not values: + errors.append(f"case {index} {field} must be a non-empty list") + elif not all(isinstance(value, str) and value.strip() for value in values): + errors.append(f"case {index} {field} must contain non-empty strings") + + if len(ids) != len(set(ids)): + errors.append("case ids must be unique") + return errors def test_ptb_skill_resources_are_available(): @@ -51,6 +106,14 @@ def test_ptb_skill_can_be_installed(tmp_path): assert (installed / expected).is_file() +@pytest.mark.parametrize("skill_name", get_packaged_skill_names()) +def test_packaged_skills_can_be_installed(skill_name, tmp_path): + installed = install_skill(skill_name, tmp_path) + + assert installed == tmp_path / skill_name + assert (installed / "SKILL.md").is_file() + + def test_ptb_skill_resources_are_packaged(tmp_path): build_output = tmp_path / "dist" result = run( @@ -92,32 +155,79 @@ def test_ptb_skill_frontmatter_is_complete(): assert "[TODO" not in content -def test_ptb_skill_passes_shared_validation(): - assert validate_skill(PTB_SKILL_NAME) == () +@pytest.mark.parametrize("skill_name", _skills_with_eval_cases()) +def test_packaged_skill_eval_cases_are_valid(skill_name): + eval_cases = _load_eval_cases(skill_name) + assert _validate_eval_cases(eval_cases, skill_name) == [] -def test_ptb_skill_eval_cases_are_valid(): - eval_cases = _load_eval_cases() - assert eval_cases["version"] == 1 - assert eval_cases["skill"] == "exasol-python-toolbox" - # Keep enough cases to cover the ticket scope, but not so many that the - # deterministic eval file becomes hard to review. - assert 6 <= len(eval_cases["cases"]) <= 8 +def _minimal_eval_cases() -> dict: + return { + "version": 1, + "skill": "example", + "cases": [ + { + "id": "case", + "category": "quality", + "prompt": "Check the API.", + "expected": { + "must_include": ["finding"], + "must_not_include": ["fix"], + }, + } + ], + } - ids = [case["id"] for case in eval_cases["cases"]] - assert len(ids) == len(set(ids)) - for case in eval_cases["cases"]: - assert case["id"] - assert case["category"] - assert case["prompt"] - assert case["expected"]["must_include"] - assert case["expected"]["must_not_include"] +@pytest.mark.parametrize( + ("change", "expected_error"), + [ + (lambda data: data.update(version=2), "version must be 1"), + ( + lambda data: data.update(skill="other"), + "skill must be example", + ), + ( + lambda data: data["cases"].clear(), + "cases must be a non-empty list", + ), + ( + lambda data: data["cases"].append(data["cases"][0].copy()), + "case ids must be unique", + ), + ( + lambda data: data["cases"][0].update(category=""), + "case 0 must have a non-empty category", + ), + ( + lambda data: data["cases"][0].update(prompt=""), + "case 0 must have a non-empty prompt", + ), + ( + lambda data: data["cases"][0].update(expected=None), + "case 0 expected must be a mapping", + ), + ( + lambda data: data["cases"][0]["expected"].update(must_include=[]), + "case 0 must_include must be a non-empty list", + ), + ( + lambda data: data["cases"][0]["expected"].update(must_not_include=[""]), + "case 0 must_not_include must contain non-empty strings", + ), + ], +) +def test_eval_case_validation_rejects_invalid_cases(change, expected_error): + eval_cases = _minimal_eval_cases() + # Each mutation represents a malformed future eval_cases.yml file. + change(eval_cases) + + assert expected_error in _validate_eval_cases(eval_cases, "example") def test_ptb_skill_eval_cases_cover_ticket_scope(): - eval_cases = _load_eval_cases() + eval_cases = _load_eval_cases(PTB_SKILL_NAME) categories = {case["category"] for case in eval_cases["cases"]} assert { @@ -131,7 +241,7 @@ def test_ptb_skill_eval_cases_cover_ticket_scope(): def test_ptb_skill_eval_cases_do_not_define_llm_ci_execution(): - content = EVAL_CASES.read_text(encoding="utf-8").lower() + content = _eval_cases_path(PTB_SKILL_NAME).read_text(encoding="utf-8").lower() forbidden = [ "model:", diff --git a/test/unit/util/skill_utils_test.py b/test/unit/util/skill_utils_test.py index 7f36cd81a..0ac7e27ff 100644 --- a/test/unit/util/skill_utils_test.py +++ b/test/unit/util/skill_utils_test.py @@ -26,7 +26,7 @@ def raise_file_not_found(_): monkeypatch.setattr(skills.resources, "files", raise_file_not_found) - with pytest.raises(RuntimeError, match="Packaged PTB skills are unavailable"): + with pytest.raises(RuntimeError, match="Packaged skills are unavailable"): skills.get_packaged_skill_names() From 27dc5adcffd9e3d268c3799ead30189693b1bee1 Mon Sep 17 00:00:00 2001 From: jana-selva Date: Tue, 29 Sep 2026 13:34:47 +0530 Subject: [PATCH 02/11] nox format fix --- test/integration/skills_integration_test.py | 1 - test/unit/skills_test.py | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/test/integration/skills_integration_test.py b/test/integration/skills_integration_test.py index 4ce440198..4ba8a52ed 100644 --- a/test/integration/skills_integration_test.py +++ b/test/integration/skills_integration_test.py @@ -1,7 +1,6 @@ from unittest.mock import Mock import noxconfig - from exasol.toolbox.nox import _skills from exasol.toolbox.util.skills import get_packaged_skill_names diff --git a/test/unit/skills_test.py b/test/unit/skills_test.py index d0f0d433c..e1d755b6c 100644 --- a/test/unit/skills_test.py +++ b/test/unit/skills_test.py @@ -1,16 +1,16 @@ +from collections.abc import Mapping from pathlib import Path from subprocess import run from zipfile import ZipFile -from collections.abc import Mapping import pytest from ruamel.yaml import YAML from exasol.toolbox.util.skills import ( PTB_SKILL_NAME, + get_packaged_skill_names, get_skill_files, get_skill_path, - get_packaged_skill_names, install_skill, ) From a4eb155a7f9e38cbe2b4847d46421f95f23b0589 Mon Sep 17 00:00:00 2001 From: jana-selva Date: Tue, 29 Sep 2026 13:45:09 +0530 Subject: [PATCH 03/11] sonar fixes --- test/unit/nox/_skills_test.py | 9 ++++++++ test/unit/util/skill_utils_test.py | 36 ++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/test/unit/nox/_skills_test.py b/test/unit/nox/_skills_test.py index f18149f3e..8c3a240af 100644 --- a/test/unit/nox/_skills_test.py +++ b/test/unit/nox/_skills_test.py @@ -61,3 +61,12 @@ def test_install_skills_uses_project_skill_directory( (("one",), {"target_directory": target_directory}), (("two",), {"target_directory": target_directory}), ] + + +def test_tasks_exports_skill_tasks(): + # Import the public task module so its exported skill sessions are covered + # in the same way users discover them through nox. + from exasol.toolbox.nox import tasks + + assert tasks.check_skills is _skills.check_skills + assert tasks.install_skills is _skills.install_skills diff --git a/test/unit/util/skill_utils_test.py b/test/unit/util/skill_utils_test.py index 0ac7e27ff..d96f254ba 100644 --- a/test/unit/util/skill_utils_test.py +++ b/test/unit/util/skill_utils_test.py @@ -115,6 +115,42 @@ def test_install_skill_rejects_path_traversal(tmp_path): skills.install_skill("../outside", tmp_path) +def test_install_skill_rejects_missing_skill(tmp_path, monkeypatch): + monkeypatch.setattr(skills, "get_skill_files", lambda _: {}) + + with pytest.raises(ValueError, match="packaged skill does not exist"): + skills.install_skill("example", tmp_path) + + +def test_install_skill_rejects_symlinked_target_directory(tmp_path, monkeypatch): + source = tmp_path / "source" + source.mkdir() + skill_file = source / "SKILL.md" + skill_file.write_text("skill", encoding="utf-8") + monkeypatch.setattr(skills, "get_skill_files", lambda _: {"SKILL.md": skill_file}) + real_target = tmp_path / "real-target" + real_target.mkdir() + target_directory = tmp_path / "linked-target" + target_directory.symlink_to(real_target, target_is_directory=True) + + with pytest.raises(ValueError, match="symlinked target directory"): + skills.install_skill("example", target_directory) + + +def test_install_skill_rejects_file_as_target(tmp_path, monkeypatch): + source = tmp_path / "source" + source.mkdir() + skill_file = source / "SKILL.md" + skill_file.write_text("skill", encoding="utf-8") + monkeypatch.setattr(skills, "get_skill_files", lambda _: {"SKILL.md": skill_file}) + target_directory = tmp_path / ".agents" / "skills" + target_directory.mkdir(parents=True) + (target_directory / "example").write_text("not a directory", encoding="utf-8") + + with pytest.raises(ValueError, match="not a directory"): + skills.install_skill("example", target_directory) + + def test_install_skill_rejects_symlink_target(tmp_path, monkeypatch): source = tmp_path / "source" source.mkdir() From c52c45a67ebd3eda70e2c04b58bc929ac223037c Mon Sep 17 00:00:00 2001 From: jana-selva Date: Wed, 30 Sep 2026 14:56:16 +0530 Subject: [PATCH 04/11] Address review feedback --- doc/changes/unreleased.md | 4 +- doc/developer_guide/agent_skills.rst | 69 +++++++++++++++ doc/developer_guide/developer_guide.rst | 1 + .../features/agent_skills/index.rst | 20 +++++ .../skills/api-contract-audit/SKILL.md | 3 +- .../skills/exasol-python-toolbox/SKILL.md | 1 + test/integration/project-template/nox_test.py | 28 ++++++ test/integration/skills_integration_test.py | 27 ------ test/unit/nox/_skills_test.py | 9 -- test/unit/skills_test.py | 86 +++++++++++-------- .../{skill_utils_test.py => skill_test.py} | 0 11 files changed, 174 insertions(+), 74 deletions(-) create mode 100644 doc/developer_guide/agent_skills.rst delete mode 100644 test/integration/skills_integration_test.py rename test/unit/util/{skill_utils_test.py => skill_test.py} (100%) diff --git a/doc/changes/unreleased.md b/doc/changes/unreleased.md index 8bdb0e172..7e971f989 100644 --- a/doc/changes/unreleased.md +++ b/doc/changes/unreleased.md @@ -3,7 +3,7 @@ ## Features * #940: Added shared validation for packaged agent skills and the `skills:check` Nox session. -* #938: Added the `skills:install` Nox session for installing packaged agent skills. -* #963: Extended packaged skill checks and installation to support multiple skills. +* #938: Added the `skills:install` Nox session for installing the packaged PTB agent skill. * #942: Added api-contract-audit skill for identifying mismatches between type annotations, docstrings, and runtime behavior +* #963: Extended packaged skill checks and installation to support multiple skills. diff --git a/doc/developer_guide/agent_skills.rst b/doc/developer_guide/agent_skills.rst new file mode 100644 index 000000000..afa65328a --- /dev/null +++ b/doc/developer_guide/agent_skills.rst @@ -0,0 +1,69 @@ +.. _developer_agent_skills: + +Testing Agent Skills +==================== + +Packaged Skills +--------------- + +The ``skills:check`` Nox session validates common structure and content rules +for every packaged skill. + +The PTB pytest suite verifies skill packaging and installation with: + +* ``test/unit/skills_test.py``: packaging, installation, required files, and + ``eval_cases.yml`` validation. +* ``test/unit/util/skill_test.py``: lower-level validation and + installation behavior. +* ``test/integration/project-template/nox_test.py``: verifies that + ``skills:check`` and ``skills:install`` operate in a newly created project. +* ``test/unit/nox/_skills_test.py``: tests Nox session behavior and failure + reporting. + +When adding a skill to the PTB: + +* Add its ``eval_cases.yml`` with representative prompts and expected response + characteristics. +* Add assertions for any files beyond ``SKILL.md`` that the skill must package + and install. +* Add deterministic, skill-specific assertions where the shared checks are + insufficient. + +These evaluation files and tests are PTB development resources; they are not +required by downstream projects using the packaged skills. + +Writing ``eval_cases.yml`` +-------------------------- + +An evaluation case should describe one distinct user goal. Keep the prompt +specific enough that a good response can be recognized from observable +evidence, and avoid requirements that merely repeat the skill's name or ask +for generic quality. + +Use ``must_include`` for concrete evidence that should appear in a correct +response, such as: + +* the names of the APIs or files that were inspected; +* the tools or checks that must be used, when they are part of the skill's + intended behavior; and +* the reported mismatch, affected API, severity, or other required result. + +Use ``must_not_include`` for concrete behavior the skill must avoid, such as +inventing findings, treating one source as authoritative without comparison, +or proposing implementation changes when the task asks only for an audit. +The prohibited text should describe an actual failure mode, not a broad word +that could legitimately occur in a response. + +For example, a signature and docstring audit can name specific existing +functions in its prompt and require evidence from +``inspect.signature()``, ``inspect.get_annotations()``, +``typing.get_type_hints()``, and ``inspect.getdoc()``. Its ``must_include`` +values can then require the inspected function names, the introspection tools, +and the concrete mismatch. This makes the case test the skill's intended +audit behavior instead of only testing whether the response is generally +relevant. + +Each case should have a unique ``id``, a meaningful ``category``, a focused +``prompt``, and non-empty ``must_include`` and ``must_not_include`` lists. +Cases should cover different behaviors rather than restating the same prompt +with minor wording changes. diff --git a/doc/developer_guide/developer_guide.rst b/doc/developer_guide/developer_guide.rst index 03037fd33..aced0a588 100644 --- a/doc/developer_guide/developer_guide.rst +++ b/doc/developer_guide/developer_guide.rst @@ -9,3 +9,4 @@ ../design plugins + agent_skills diff --git a/doc/user_guide/features/agent_skills/index.rst b/doc/user_guide/features/agent_skills/index.rst index 7b9d5980c..55870ff0d 100644 --- a/doc/user_guide/features/agent_skills/index.rst +++ b/doc/user_guide/features/agent_skills/index.rst @@ -6,6 +6,26 @@ Agent Skills The PTB can package agent skills for use by projects and provides shared validation for their common structure and content rules. +Packaged skills maintained by the PTB +------------------------------------- + +The following skill directories are provided by the PTB. Installing packaged +skills can replace a project-local skill with the same name, so choose local +skill names with this list in mind. + +.. list-table:: + :widths: 30 70 + :header-rows: 1 + + * - Skill directory + - Intended use + * - ``api-contract-audit`` + - Audits a Python library for mismatches between type annotations, + docstrings, user-facing documentation, and runtime behavior. + * - ``exasol-python-toolbox`` + - Guides agents in using PTB setup, Nox sessions, checks, workflows, + updates, releases, and configuration. + Run the validation with: .. code-block:: shell diff --git a/exasol/toolbox/skills/api-contract-audit/SKILL.md b/exasol/toolbox/skills/api-contract-audit/SKILL.md index de5e6f77e..3f76f6cd6 100644 --- a/exasol/toolbox/skills/api-contract-audit/SKILL.md +++ b/exasol/toolbox/skills/api-contract-audit/SKILL.md @@ -1,4 +1,5 @@ --- +# Generated and maintained by the exasol-toolbox. name: api-contract-audit description: Audit a Python library's public API for inconsistencies between type annotations, docstrings, user-facing documentation/examples, and actual runtime behavior. Use when reviewing API changes, checking whether public methods accept undocumented parameter shapes, or validating that docs and type hints match enforcement in code. --- @@ -87,4 +88,4 @@ If no findings are discovered, say that explicitly and mention any coverage limi - Do not rewrite the API contract on your own. If code, docs, and examples disagree, report the disagreement. - Do not stop at the first example. Check for the same pattern across sibling APIs. -- Do not treat private helper inconsistencies as findings unless they affect public behavior. \ No newline at end of file +- Do not treat private helper inconsistencies as findings unless they affect public behavior. diff --git a/exasol/toolbox/skills/exasol-python-toolbox/SKILL.md b/exasol/toolbox/skills/exasol-python-toolbox/SKILL.md index e9d6ee49a..523e54a50 100644 --- a/exasol/toolbox/skills/exasol-python-toolbox/SKILL.md +++ b/exasol/toolbox/skills/exasol-python-toolbox/SKILL.md @@ -1,4 +1,5 @@ --- +# Generated and maintained by the exasol-toolbox. name: exasol-python-toolbox description: Use this skill in Exasol Python projects that use exasol-toolbox/PTB. Use it for PTB setup, nox sessions, code checks, GitHub workflows, updates, releases, and PTB configuration. Use it when an agent must not replace PTB automation. --- diff --git a/test/integration/project-template/nox_test.py b/test/integration/project-template/nox_test.py index 994e998fa..1dbaf685d 100644 --- a/test/integration/project-template/nox_test.py +++ b/test/integration/project-template/nox_test.py @@ -1,5 +1,10 @@ from exasol.toolbox.util.version import Version +EXPECTED_PACKAGED_SKILL_FILE_COUNTS = { + "api-contract-audit": 1, + "exasol-python-toolbox": 5, +} + class TestSpecificNoxTasks: """ @@ -84,3 +89,26 @@ def test_install_github_workflows(self, poetry_path, run_command): file_list = run_command(["ls", ".github/workflows"]).stdout.splitlines() assert len(file_list) == 14 + + def test_skills_install_and_check( + self, poetry_path, run_command, new_project + ): + skills_install = self._command(poetry_path, "skills:install") + run_command(skills_install) + + installed_skills = new_project / ".agents" / "skills" + installed_skill_names = { + path.name for path in installed_skills.iterdir() if path.is_dir() + } + assert set(EXPECTED_PACKAGED_SKILL_FILE_COUNTS) <= installed_skill_names + + for skill_name, expected_file_count in EXPECTED_PACKAGED_SKILL_FILE_COUNTS.items(): + installed_file_count = sum( + path.is_file() for path in (installed_skills / skill_name).rglob("*") + ) + assert installed_file_count == expected_file_count + + skills_check = self._command(poetry_path, "skills:check") + output = run_command(skills_check) + + assert output.returncode == 0 diff --git a/test/integration/skills_integration_test.py b/test/integration/skills_integration_test.py deleted file mode 100644 index 4ba8a52ed..000000000 --- a/test/integration/skills_integration_test.py +++ /dev/null @@ -1,27 +0,0 @@ -from unittest.mock import Mock - -import noxconfig -from exasol.toolbox.nox import _skills -from exasol.toolbox.util.skills import get_packaged_skill_names - - -def test_skills_check_validates_all_packaged_skills(): - # Use real package resources here; unit tests separately cover failure - # formatting with mocked skill names and validation results. - packaged_skills = get_packaged_skill_names() - - assert {"api-contract-audit", "exasol-python-toolbox"}.issubset(packaged_skills) - - session = Mock() - _skills.check_skills(session) - - session.error.assert_not_called() - - -def test_skills_install_installs_all_packaged_skills(tmp_path, monkeypatch): - monkeypatch.setattr(noxconfig, "PROJECT_CONFIG", Mock(agent_skills_path=tmp_path)) - - _skills.install_skills(Mock()) - - for skill_name in get_packaged_skill_names(): - assert (tmp_path / skill_name / "SKILL.md").is_file() diff --git a/test/unit/nox/_skills_test.py b/test/unit/nox/_skills_test.py index 8c3a240af..f18149f3e 100644 --- a/test/unit/nox/_skills_test.py +++ b/test/unit/nox/_skills_test.py @@ -61,12 +61,3 @@ def test_install_skills_uses_project_skill_directory( (("one",), {"target_directory": target_directory}), (("two",), {"target_directory": target_directory}), ] - - -def test_tasks_exports_skill_tasks(): - # Import the public task module so its exported skill sessions are covered - # in the same way users discover them through nox. - from exasol.toolbox.nox import tasks - - assert tasks.check_skills is _skills.check_skills - assert tasks.install_skills is _skills.install_skills diff --git a/test/unit/skills_test.py b/test/unit/skills_test.py index e1d755b6c..4f7d26a19 100644 --- a/test/unit/skills_test.py +++ b/test/unit/skills_test.py @@ -156,10 +156,11 @@ def test_ptb_skill_frontmatter_is_complete(): @pytest.mark.parametrize("skill_name", _skills_with_eval_cases()) -def test_packaged_skill_eval_cases_are_valid(skill_name): - eval_cases = _load_eval_cases(skill_name) +class TestPackagedSkillEvalCases: + def test_schema_is_valid(self, skill_name): + eval_cases = _load_eval_cases(skill_name) - assert _validate_eval_cases(eval_cases, skill_name) == [] + assert _validate_eval_cases(eval_cases, skill_name) == [] def _minimal_eval_cases() -> dict: @@ -180,50 +181,65 @@ def _minimal_eval_cases() -> dict: } -@pytest.mark.parametrize( - ("change", "expected_error"), - [ - (lambda data: data.update(version=2), "version must be 1"), - ( - lambda data: data.update(skill="other"), - "skill must be example", - ), - ( - lambda data: data["cases"].clear(), - "cases must be a non-empty list", - ), - ( +class TestEvalCaseValidation: + @staticmethod + def _assert_rejected(change, expected_error): + eval_cases = _minimal_eval_cases() + # Each mutation represents a malformed future eval_cases.yml file. + change(eval_cases) + + assert expected_error in _validate_eval_cases(eval_cases, "example") + + def test_rejects_invalid_version(self): + self._assert_rejected( + lambda data: data.update(version=2), "version must be 1" + ) + + def test_rejects_invalid_skill_name(self): + self._assert_rejected( + lambda data: data.update(skill="other"), "skill must be example" + ) + + def test_rejects_empty_cases(self): + self._assert_rejected( + lambda data: data["cases"].clear(), "cases must be a non-empty list" + ) + + def test_rejects_duplicate_case_ids(self): + self._assert_rejected( lambda data: data["cases"].append(data["cases"][0].copy()), "case ids must be unique", - ), - ( + ) + + def test_rejects_empty_category(self): + self._assert_rejected( lambda data: data["cases"][0].update(category=""), "case 0 must have a non-empty category", - ), - ( + ) + + def test_rejects_empty_prompt(self): + self._assert_rejected( lambda data: data["cases"][0].update(prompt=""), "case 0 must have a non-empty prompt", - ), - ( + ) + + def test_rejects_missing_expected_mapping(self): + self._assert_rejected( lambda data: data["cases"][0].update(expected=None), "case 0 expected must be a mapping", - ), - ( + ) + + def test_rejects_empty_must_include(self): + self._assert_rejected( lambda data: data["cases"][0]["expected"].update(must_include=[]), "case 0 must_include must be a non-empty list", - ), - ( + ) + + def test_rejects_blank_must_not_include(self): + self._assert_rejected( lambda data: data["cases"][0]["expected"].update(must_not_include=[""]), "case 0 must_not_include must contain non-empty strings", - ), - ], -) -def test_eval_case_validation_rejects_invalid_cases(change, expected_error): - eval_cases = _minimal_eval_cases() - # Each mutation represents a malformed future eval_cases.yml file. - change(eval_cases) - - assert expected_error in _validate_eval_cases(eval_cases, "example") + ) def test_ptb_skill_eval_cases_cover_ticket_scope(): diff --git a/test/unit/util/skill_utils_test.py b/test/unit/util/skill_test.py similarity index 100% rename from test/unit/util/skill_utils_test.py rename to test/unit/util/skill_test.py From 377e716ab82cea69f60b0d3b94cd738bb150b6b6 Mon Sep 17 00:00:00 2001 From: jana-selva Date: Wed, 30 Sep 2026 14:58:00 +0530 Subject: [PATCH 05/11] format fix nox --- test/integration/project-template/nox_test.py | 9 +++++---- test/unit/skills_test.py | 4 +--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/test/integration/project-template/nox_test.py b/test/integration/project-template/nox_test.py index 1dbaf685d..9cfd87927 100644 --- a/test/integration/project-template/nox_test.py +++ b/test/integration/project-template/nox_test.py @@ -90,9 +90,7 @@ def test_install_github_workflows(self, poetry_path, run_command): file_list = run_command(["ls", ".github/workflows"]).stdout.splitlines() assert len(file_list) == 14 - def test_skills_install_and_check( - self, poetry_path, run_command, new_project - ): + def test_skills_install_and_check(self, poetry_path, run_command, new_project): skills_install = self._command(poetry_path, "skills:install") run_command(skills_install) @@ -102,7 +100,10 @@ def test_skills_install_and_check( } assert set(EXPECTED_PACKAGED_SKILL_FILE_COUNTS) <= installed_skill_names - for skill_name, expected_file_count in EXPECTED_PACKAGED_SKILL_FILE_COUNTS.items(): + for ( + skill_name, + expected_file_count, + ) in EXPECTED_PACKAGED_SKILL_FILE_COUNTS.items(): installed_file_count = sum( path.is_file() for path in (installed_skills / skill_name).rglob("*") ) diff --git a/test/unit/skills_test.py b/test/unit/skills_test.py index 4f7d26a19..91a9628d0 100644 --- a/test/unit/skills_test.py +++ b/test/unit/skills_test.py @@ -191,9 +191,7 @@ def _assert_rejected(change, expected_error): assert expected_error in _validate_eval_cases(eval_cases, "example") def test_rejects_invalid_version(self): - self._assert_rejected( - lambda data: data.update(version=2), "version must be 1" - ) + self._assert_rejected(lambda data: data.update(version=2), "version must be 1") def test_rejects_invalid_skill_name(self): self._assert_rejected( From e224363384ec899c44d043ef0853f6da81490059 Mon Sep 17 00:00:00 2001 From: jana-selva Date: Wed, 30 Sep 2026 15:11:09 +0530 Subject: [PATCH 06/11] Improve test coverage --- test/unit/nox/_documentation_test.py | 94 ++++++++++++++++++++++++++-- test/unit/nox/_test_test.py | 22 ++++++- test/unit/nox/tasks_test.py | 24 +++++++ 3 files changed, 135 insertions(+), 5 deletions(-) create mode 100644 test/unit/nox/tasks_test.py diff --git a/test/unit/nox/_documentation_test.py b/test/unit/nox/_documentation_test.py index c7b8261cf..408343b33 100644 --- a/test/unit/nox/_documentation_test.py +++ b/test/unit/nox/_documentation_test.py @@ -1,17 +1,22 @@ import shutil -from unittest.mock import ( - MagicMock, - patch, -) +from unittest.mock import MagicMock, Mock, patch import pytest from nox.sessions import _SessionQuit from exasol.toolbox.nox._documentation import ( + _build_docs, + _build_multiversion_docs, + _docs_list_links, _docs_links_check, + build_docs, + build_multiversion, + clean_docs, docs_links_check, docs_list_links, + open_docs, ) +from exasol.toolbox.nox._shared import DOCS_OUTPUT_DIR from noxconfig import PROJECT_CONFIG @@ -72,6 +77,87 @@ def test_raises_error_for_rcode_not_0(nox_session, config): docs_list_links(nox_session) +def test_build_docs_runs_sphinx(nox_session, config): + with patch("exasol.toolbox.nox._documentation._build_docs") as build: + with patch("exasol.toolbox.nox._documentation.PROJECT_CONFIG", new=config): + build_docs(nox_session) + + build.assert_called_once_with(nox_session, config) + + +def test_build_multiversion_docs_runs_sphinx(nox_session, config): + with patch( + "exasol.toolbox.nox._documentation._build_multiversion_docs" + ) as build: + with patch("exasol.toolbox.nox._documentation.PROJECT_CONFIG", new=config): + build_multiversion(nox_session) + + build.assert_called_once_with(nox_session, config) + + +def test_build_docs_command(config): + session = Mock() + _build_docs(session, config) + + session.run.assert_called_once_with( + "sphinx-build", + "-W", + "-b", + "html", + f"{config.documentation_path}", + DOCS_OUTPUT_DIR, + ) + + +def test_build_multiversion_docs_commands(config): + session = Mock() + _build_multiversion_docs(session, config) + + assert session.run.call_count == 2 + session.run.assert_any_call( + "sphinx-multiversion", + f"{config.documentation_path}", + DOCS_OUTPUT_DIR, + ) + session.run.assert_any_call("touch", f"{DOCS_OUTPUT_DIR}/.nojekyll") + + +def test_open_docs_reports_missing_output(nox_session, config): + with patch("exasol.toolbox.nox._documentation.PROJECT_CONFIG", new=config): + with pytest.raises(_SessionQuit): + open_docs(nox_session) + + +def test_open_docs_opens_index(nox_session, config): + docs_folder = config.root_path / DOCS_OUTPUT_DIR + docs_folder.mkdir() + (docs_folder / "index.html").touch() + with patch("exasol.toolbox.nox._documentation.PROJECT_CONFIG", new=config): + with patch("exasol.toolbox.nox._documentation.webbrowser.open_new_tab") as open_tab: + open_docs(nox_session) + + open_tab.assert_called_once_with((docs_folder / "index.html").as_uri()) + + +def test_clean_docs_removes_output(nox_session, config): + docs_folder = config.root_path / DOCS_OUTPUT_DIR + docs_folder.mkdir() + with patch("exasol.toolbox.nox._documentation.PROJECT_CONFIG", new=config): + clean_docs(nox_session) + + assert not docs_folder.exists() + + +def test_docs_list_links_reports_sphinx_failure(tmp_path): + with patch( + "exasol.toolbox.nox._documentation.subprocess.run", + return_value=MagicMock(returncode=2, stderr="sphinx failed"), + ): + result = _docs_list_links(tmp_path) + + assert result == (2, "sphinx failed") + + @pytest.mark.slow @pytest.mark.parametrize( "file_content, expected_code, expected_message", diff --git a/test/unit/nox/_test_test.py b/test/unit/nox/_test_test.py index c44a347cd..d689f88bf 100644 --- a/test/unit/nox/_test_test.py +++ b/test/unit/nox/_test_test.py @@ -1,9 +1,10 @@ import shutil -from unittest.mock import patch +from unittest.mock import Mock, patch import pytest from exasol.toolbox.nox._test import ( + _coverage, _test_command, coverage, integration_tests, @@ -136,3 +137,22 @@ def test_coverage_uses_integration_test_context(nox_session): PROJECT_CONFIG, {"coverage": True, "db_version": "8.29.13", "fwd-args": []}, ) + + +def test_coverage_runs_tests_and_reports(tmp_path, test_project_config_factory): + config = test_project_config_factory(root_path=tmp_path) + coverage_file = tmp_path / ".coverage" + coverage_file.touch() + context = {"coverage": True, "db_version": "8.29.13", "fwd-args": []} + session = Mock() + + with ( + patch("exasol.toolbox.nox._test._unit_tests") as unit, + patch("exasol.toolbox.nox._test._integration_tests") as integration, + ): + _coverage(session, config, context) + + assert not coverage_file.exists() + unit.assert_called_once_with(session, config, context) + integration.assert_called_once_with(session, config, context) + session.run.assert_called_once_with("coverage", "report", "-m") diff --git a/test/unit/nox/tasks_test.py b/test/unit/nox/tasks_test.py new file mode 100644 index 000000000..4a4c594ca --- /dev/null +++ b/test/unit/nox/tasks_test.py @@ -0,0 +1,24 @@ +from unittest.mock import Mock, patch + +from exasol.toolbox.nox import tasks + + +def test_check_runs_all_project_checks(): + session = Mock() + context = {"coverage": True, "db_version": "8.29.13", "fwd-args": []} + files = ("example.py",) + + with ( + patch.object(tasks, "_integration_test_context", return_value=context), + patch.object(tasks, "get_filtered_python_files", return_value=files), + patch.object(tasks, "_code_format") as code_format, + patch.object(tasks, "_pylint") as pylint, + patch.object(tasks, "_type_check") as type_check, + patch.object(tasks, "_coverage") as coverage, + ): + tasks.check(session) + + code_format.assert_called_once_with(session, tasks.Mode.Check, files) + pylint.assert_called_once_with(session, files) + type_check.assert_called_once_with(session, files) + coverage.assert_called_once_with(session, tasks.PROJECT_CONFIG, context) From a57eb97e1d8f9617ed773831dda5bdf4bb903b53 Mon Sep 17 00:00:00 2001 From: jana-selva Date: Wed, 30 Sep 2026 15:12:35 +0530 Subject: [PATCH 07/11] sonar fixes and format fixes --- test/unit/nox/_documentation_test.py | 16 ++++++++++------ test/unit/nox/_test_test.py | 5 ++++- test/unit/nox/tasks_test.py | 5 ++++- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/test/unit/nox/_documentation_test.py b/test/unit/nox/_documentation_test.py index 408343b33..85d0a2d37 100644 --- a/test/unit/nox/_documentation_test.py +++ b/test/unit/nox/_documentation_test.py @@ -1,5 +1,9 @@ import shutil -from unittest.mock import MagicMock, Mock, patch +from unittest.mock import ( + MagicMock, + Mock, + patch, +) import pytest from nox.sessions import _SessionQuit @@ -7,8 +11,8 @@ from exasol.toolbox.nox._documentation import ( _build_docs, _build_multiversion_docs, - _docs_list_links, _docs_links_check, + _docs_list_links, build_docs, build_multiversion, clean_docs, @@ -86,9 +90,7 @@ def test_build_docs_runs_sphinx(nox_session, config): def test_build_multiversion_docs_runs_sphinx(nox_session, config): - with patch( - "exasol.toolbox.nox._documentation._build_multiversion_docs" - ) as build: + with patch("exasol.toolbox.nox._documentation._build_multiversion_docs") as build: with patch("exasol.toolbox.nox._documentation.PROJECT_CONFIG", new=config): build_multiversion(nox_session) @@ -133,7 +135,9 @@ def test_open_docs_opens_index(nox_session, config): docs_folder.mkdir() (docs_folder / "index.html").touch() with patch("exasol.toolbox.nox._documentation.PROJECT_CONFIG", new=config): - with patch("exasol.toolbox.nox._documentation.webbrowser.open_new_tab") as open_tab: + with patch( + "exasol.toolbox.nox._documentation.webbrowser.open_new_tab" + ) as open_tab: open_docs(nox_session) open_tab.assert_called_once_with((docs_folder / "index.html").as_uri()) diff --git a/test/unit/nox/_test_test.py b/test/unit/nox/_test_test.py index d689f88bf..341704de2 100644 --- a/test/unit/nox/_test_test.py +++ b/test/unit/nox/_test_test.py @@ -1,5 +1,8 @@ import shutil -from unittest.mock import Mock, patch +from unittest.mock import ( + Mock, + patch, +) import pytest diff --git a/test/unit/nox/tasks_test.py b/test/unit/nox/tasks_test.py index 4a4c594ca..662f2c677 100644 --- a/test/unit/nox/tasks_test.py +++ b/test/unit/nox/tasks_test.py @@ -1,4 +1,7 @@ -from unittest.mock import Mock, patch +from unittest.mock import ( + Mock, + patch, +) from exasol.toolbox.nox import tasks From b81a6eb3faacc93baf5e45261a7c184639aef831 Mon Sep 17 00:00:00 2001 From: jana-selva Date: Mon, 5 Oct 2026 12:43:34 +0530 Subject: [PATCH 08/11] Improve skill tests --- test/integration/project-template/nox_test.py | 47 ++++++ test/unit/nox/_documentation_test.py | 90 ---------- test/unit/nox/_test_test.py | 25 +-- test/unit/nox/tasks_test.py | 27 --- test/unit/skills_test.py | 158 ++++-------------- 5 files changed, 78 insertions(+), 269 deletions(-) delete mode 100644 test/unit/nox/tasks_test.py diff --git a/test/integration/project-template/nox_test.py b/test/integration/project-template/nox_test.py index 9cfd87927..f42751e5a 100644 --- a/test/integration/project-template/nox_test.py +++ b/test/integration/project-template/nox_test.py @@ -4,6 +4,43 @@ "api-contract-audit": 1, "exasol-python-toolbox": 5, } +EXPECTED_NOX_SESSIONS = { + "format:fix", + "format:check", + "project:check", + "test:unit", + "test:integration", + "test:coverage", + "lint:code", + "lint:typing", + "lint:security", + "lint:dependencies", + "docs:multiversion", + "docs:build", + "docs:open", + "docs:clean", + "links:list", + "links:check", + "changelog:updated", + "release:prepare", + "release:update", + "release:trigger", + "skills:check", + "skills:install", + "matrix:generate", + "artifacts:validate", + "artifacts:copy", + "sonar:check", + "dependency:licenses", + "dependency:audit", + "vulnerabilities:update", + "vulnerabilities:resolved", + "dependency:sbom", + "package:check", + "workflow:check", + "workflow:generate", + "workflow:audit", +} class TestSpecificNoxTasks: @@ -113,3 +150,13 @@ def test_skills_install_and_check(self, poetry_path, run_command, new_project): output = run_command(skills_check) assert output.returncode == 0 + + def test_exposed_nox_sessions(self, poetry_path, run_command): + output = run_command([poetry_path, "run", "--", "nox", "-l"]) + sessions = { + line[2:].split(" ->", maxsplit=1)[0] + for line in output.stdout.splitlines() + if line.startswith(("* ", "- ")) + } + + assert sessions == EXPECTED_NOX_SESSIONS diff --git a/test/unit/nox/_documentation_test.py b/test/unit/nox/_documentation_test.py index 85d0a2d37..c7b8261cf 100644 --- a/test/unit/nox/_documentation_test.py +++ b/test/unit/nox/_documentation_test.py @@ -1,7 +1,6 @@ import shutil from unittest.mock import ( MagicMock, - Mock, patch, ) @@ -9,18 +8,10 @@ from nox.sessions import _SessionQuit from exasol.toolbox.nox._documentation import ( - _build_docs, - _build_multiversion_docs, _docs_links_check, - _docs_list_links, - build_docs, - build_multiversion, - clean_docs, docs_links_check, docs_list_links, - open_docs, ) -from exasol.toolbox.nox._shared import DOCS_OUTPUT_DIR from noxconfig import PROJECT_CONFIG @@ -81,87 +72,6 @@ def test_raises_error_for_rcode_not_0(nox_session, config): docs_list_links(nox_session) -def test_build_docs_runs_sphinx(nox_session, config): - with patch("exasol.toolbox.nox._documentation._build_docs") as build: - with patch("exasol.toolbox.nox._documentation.PROJECT_CONFIG", new=config): - build_docs(nox_session) - - build.assert_called_once_with(nox_session, config) - - -def test_build_multiversion_docs_runs_sphinx(nox_session, config): - with patch("exasol.toolbox.nox._documentation._build_multiversion_docs") as build: - with patch("exasol.toolbox.nox._documentation.PROJECT_CONFIG", new=config): - build_multiversion(nox_session) - - build.assert_called_once_with(nox_session, config) - - -def test_build_docs_command(config): - session = Mock() - _build_docs(session, config) - - session.run.assert_called_once_with( - "sphinx-build", - "-W", - "-b", - "html", - f"{config.documentation_path}", - DOCS_OUTPUT_DIR, - ) - - -def test_build_multiversion_docs_commands(config): - session = Mock() - _build_multiversion_docs(session, config) - - assert session.run.call_count == 2 - session.run.assert_any_call( - "sphinx-multiversion", - f"{config.documentation_path}", - DOCS_OUTPUT_DIR, - ) - session.run.assert_any_call("touch", f"{DOCS_OUTPUT_DIR}/.nojekyll") - - -def test_open_docs_reports_missing_output(nox_session, config): - with patch("exasol.toolbox.nox._documentation.PROJECT_CONFIG", new=config): - with pytest.raises(_SessionQuit): - open_docs(nox_session) - - -def test_open_docs_opens_index(nox_session, config): - docs_folder = config.root_path / DOCS_OUTPUT_DIR - docs_folder.mkdir() - (docs_folder / "index.html").touch() - with patch("exasol.toolbox.nox._documentation.PROJECT_CONFIG", new=config): - with patch( - "exasol.toolbox.nox._documentation.webbrowser.open_new_tab" - ) as open_tab: - open_docs(nox_session) - - open_tab.assert_called_once_with((docs_folder / "index.html").as_uri()) - - -def test_clean_docs_removes_output(nox_session, config): - docs_folder = config.root_path / DOCS_OUTPUT_DIR - docs_folder.mkdir() - with patch("exasol.toolbox.nox._documentation.PROJECT_CONFIG", new=config): - clean_docs(nox_session) - - assert not docs_folder.exists() - - -def test_docs_list_links_reports_sphinx_failure(tmp_path): - with patch( - "exasol.toolbox.nox._documentation.subprocess.run", - return_value=MagicMock(returncode=2, stderr="sphinx failed"), - ): - result = _docs_list_links(tmp_path) - - assert result == (2, "sphinx failed") - - @pytest.mark.slow @pytest.mark.parametrize( "file_content, expected_code, expected_message", diff --git a/test/unit/nox/_test_test.py b/test/unit/nox/_test_test.py index 341704de2..c44a347cd 100644 --- a/test/unit/nox/_test_test.py +++ b/test/unit/nox/_test_test.py @@ -1,13 +1,9 @@ import shutil -from unittest.mock import ( - Mock, - patch, -) +from unittest.mock import patch import pytest from exasol.toolbox.nox._test import ( - _coverage, _test_command, coverage, integration_tests, @@ -140,22 +136,3 @@ def test_coverage_uses_integration_test_context(nox_session): PROJECT_CONFIG, {"coverage": True, "db_version": "8.29.13", "fwd-args": []}, ) - - -def test_coverage_runs_tests_and_reports(tmp_path, test_project_config_factory): - config = test_project_config_factory(root_path=tmp_path) - coverage_file = tmp_path / ".coverage" - coverage_file.touch() - context = {"coverage": True, "db_version": "8.29.13", "fwd-args": []} - session = Mock() - - with ( - patch("exasol.toolbox.nox._test._unit_tests") as unit, - patch("exasol.toolbox.nox._test._integration_tests") as integration, - ): - _coverage(session, config, context) - - assert not coverage_file.exists() - unit.assert_called_once_with(session, config, context) - integration.assert_called_once_with(session, config, context) - session.run.assert_called_once_with("coverage", "report", "-m") diff --git a/test/unit/nox/tasks_test.py b/test/unit/nox/tasks_test.py deleted file mode 100644 index 662f2c677..000000000 --- a/test/unit/nox/tasks_test.py +++ /dev/null @@ -1,27 +0,0 @@ -from unittest.mock import ( - Mock, - patch, -) - -from exasol.toolbox.nox import tasks - - -def test_check_runs_all_project_checks(): - session = Mock() - context = {"coverage": True, "db_version": "8.29.13", "fwd-args": []} - files = ("example.py",) - - with ( - patch.object(tasks, "_integration_test_context", return_value=context), - patch.object(tasks, "get_filtered_python_files", return_value=files), - patch.object(tasks, "_code_format") as code_format, - patch.object(tasks, "_pylint") as pylint, - patch.object(tasks, "_type_check") as type_check, - patch.object(tasks, "_coverage") as coverage, - ): - tasks.check(session) - - code_format.assert_called_once_with(session, tasks.Mode.Check, files) - pylint.assert_called_once_with(session, files) - type_check.assert_called_once_with(session, files) - coverage.assert_called_once_with(session, tasks.PROJECT_CONFIG, context) diff --git a/test/unit/skills_test.py b/test/unit/skills_test.py index 91a9628d0..3e01bad33 100644 --- a/test/unit/skills_test.py +++ b/test/unit/skills_test.py @@ -1,4 +1,3 @@ -from collections.abc import Mapping from pathlib import Path from subprocess import run from zipfile import ZipFile @@ -44,52 +43,6 @@ def _skills_with_eval_cases() -> list[str]: ] -def _validate_eval_cases(eval_cases: object, skill_name: str) -> list[str]: - # Eval cases are test resources, so validate their reusable schema here - # instead of coupling production skill discovery to test-only files. - errors: list[str] = [] - if not isinstance(eval_cases, Mapping): - return ["evaluation cases must be a mapping"] - if eval_cases.get("version") != 1: - errors.append("version must be 1") - if eval_cases.get("skill") != skill_name: - errors.append(f"skill must be {skill_name}") - - cases = eval_cases.get("cases") - if not isinstance(cases, list) or not cases: - return errors + ["cases must be a non-empty list"] - - ids: list[str] = [] - for index, case in enumerate(cases): - if not isinstance(case, Mapping): - errors.append(f"case {index} must be a mapping") - continue - case_id = case.get("id") - if not isinstance(case_id, str) or not case_id.strip(): - errors.append(f"case {index} must have a non-empty id") - else: - ids.append(case_id) - for field in ("category", "prompt"): - value = case.get(field) - if not isinstance(value, str) or not value.strip(): - errors.append(f"case {index} must have a non-empty {field}") - - expected = case.get("expected") - if not isinstance(expected, Mapping): - errors.append(f"case {index} expected must be a mapping") - continue - for field in ("must_include", "must_not_include"): - values = expected.get(field) - if not isinstance(values, list) or not values: - errors.append(f"case {index} {field} must be a non-empty list") - elif not all(isinstance(value, str) and value.strip() for value in values): - errors.append(f"case {index} {field} must contain non-empty strings") - - if len(ids) != len(set(ids)): - errors.append("case ids must be unique") - return errors - - def test_ptb_skill_resources_are_available(): skill_files = get_skill_files(PTB_SKILL_NAME) @@ -157,87 +110,36 @@ def test_ptb_skill_frontmatter_is_complete(): @pytest.mark.parametrize("skill_name", _skills_with_eval_cases()) class TestPackagedSkillEvalCases: - def test_schema_is_valid(self, skill_name): - eval_cases = _load_eval_cases(skill_name) - - assert _validate_eval_cases(eval_cases, skill_name) == [] - - -def _minimal_eval_cases() -> dict: - return { - "version": 1, - "skill": "example", - "cases": [ - { - "id": "case", - "category": "quality", - "prompt": "Check the API.", - "expected": { - "must_include": ["finding"], - "must_not_include": ["fix"], - }, - } - ], - } - - -class TestEvalCaseValidation: - @staticmethod - def _assert_rejected(change, expected_error): - eval_cases = _minimal_eval_cases() - # Each mutation represents a malformed future eval_cases.yml file. - change(eval_cases) - - assert expected_error in _validate_eval_cases(eval_cases, "example") - - def test_rejects_invalid_version(self): - self._assert_rejected(lambda data: data.update(version=2), "version must be 1") - - def test_rejects_invalid_skill_name(self): - self._assert_rejected( - lambda data: data.update(skill="other"), "skill must be example" - ) - - def test_rejects_empty_cases(self): - self._assert_rejected( - lambda data: data["cases"].clear(), "cases must be a non-empty list" - ) - - def test_rejects_duplicate_case_ids(self): - self._assert_rejected( - lambda data: data["cases"].append(data["cases"][0].copy()), - "case ids must be unique", - ) - - def test_rejects_empty_category(self): - self._assert_rejected( - lambda data: data["cases"][0].update(category=""), - "case 0 must have a non-empty category", - ) - - def test_rejects_empty_prompt(self): - self._assert_rejected( - lambda data: data["cases"][0].update(prompt=""), - "case 0 must have a non-empty prompt", - ) - - def test_rejects_missing_expected_mapping(self): - self._assert_rejected( - lambda data: data["cases"][0].update(expected=None), - "case 0 expected must be a mapping", - ) - - def test_rejects_empty_must_include(self): - self._assert_rejected( - lambda data: data["cases"][0]["expected"].update(must_include=[]), - "case 0 must_include must be a non-empty list", - ) - - def test_rejects_blank_must_not_include(self): - self._assert_rejected( - lambda data: data["cases"][0]["expected"].update(must_not_include=[""]), - "case 0 must_not_include must contain non-empty strings", - ) + @pytest.fixture + def eval_cases(self, skill_name): + # Load the packaged artifact once so all checks inspect the same data. + return _load_eval_cases(skill_name) + + def test_has_expected_metadata(self, eval_cases, skill_name): + + assert eval_cases["version"] == 1 + assert eval_cases["skill"] == skill_name + assert isinstance(eval_cases["cases"], list) + assert eval_cases["cases"] + + def test_cases_have_required_fields(self, eval_cases): + for case in eval_cases["cases"]: + assert case["id"].strip() + assert case["category"].strip() + assert case["prompt"].strip() + + def test_cases_have_response_constraints(self, eval_cases): + for case in eval_cases["cases"]: + expected = case["expected"] + assert expected["must_include"] + assert expected["must_not_include"] + assert all(value.strip() for value in expected["must_include"]) + assert all(value.strip() for value in expected["must_not_include"]) + + def test_case_ids_are_unique(self, eval_cases): + ids = [case["id"] for case in eval_cases["cases"]] + + assert len(ids) == len(set(ids)) def test_ptb_skill_eval_cases_cover_ticket_scope(): From d1f673a8a5395e8e4878ad2a9051f1c6e5facd18 Mon Sep 17 00:00:00 2001 From: jana-selva Date: Wed, 7 Oct 2026 14:14:09 +0530 Subject: [PATCH 09/11] Validate packaged skill evaluation cases --- doc/api/index.rst | 1 + doc/api/skill_eval.rst | 10 +++++++ exasol/toolbox/util/skill_eval.py | 38 ++++++++++++++++++++++++++ test/unit/skills_test.py | 45 ++++++++++++++++++------------- 4 files changed, 75 insertions(+), 19 deletions(-) create mode 100644 doc/api/skill_eval.rst create mode 100644 exasol/toolbox/util/skill_eval.py diff --git a/doc/api/index.rst b/doc/api/index.rst index 8ad43c961..3b9fc2209 100644 --- a/doc/api/index.rst +++ b/doc/api/index.rst @@ -7,5 +7,6 @@ :maxdepth: 2 base_config + skill_eval workflow_exceptions workflow_patcher_config diff --git a/doc/api/skill_eval.rst b/doc/api/skill_eval.rst new file mode 100644 index 000000000..454101aa8 --- /dev/null +++ b/doc/api/skill_eval.rst @@ -0,0 +1,10 @@ +.. _skill_eval: + +Skill Evaluation Cases +---------------------- + +These models define the structure of the ``eval_cases.yml`` file packaged with +an agent skill. + +.. automodule:: exasol.toolbox.util.skill_eval + :members: diff --git a/exasol/toolbox/util/skill_eval.py b/exasol/toolbox/util/skill_eval.py new file mode 100644 index 000000000..2cbebb5f6 --- /dev/null +++ b/exasol/toolbox/util/skill_eval.py @@ -0,0 +1,38 @@ +"""Models for validating packaged agent-skill evaluation cases.""" + +from typing import Annotated, Literal + +from pydantic import BaseModel, ConfigDict, Field + + +NonBlankString = Annotated[str, Field(pattern=r"\S")] + + +class ExpectedResponse(BaseModel): + """Required and forbidden content for one evaluation response.""" + + model_config = ConfigDict(extra="forbid") + + must_include: list[NonBlankString] = Field(min_length=1) + must_not_include: list[NonBlankString] = Field(min_length=1) + + +class EvalCase(BaseModel): + """One prompt and its expected response constraints.""" + + model_config = ConfigDict(extra="forbid") + + id: NonBlankString + category: NonBlankString + prompt: NonBlankString + expected: ExpectedResponse + + +class PackagedSkillEvalCases(BaseModel): + """Schema for a packaged skill's ``eval_cases.yml`` file.""" + + model_config = ConfigDict(extra="forbid") + + version: Literal[1] + skill: NonBlankString + cases: list[EvalCase] = Field(min_length=1) diff --git a/test/unit/skills_test.py b/test/unit/skills_test.py index 3e01bad33..925e62c95 100644 --- a/test/unit/skills_test.py +++ b/test/unit/skills_test.py @@ -5,6 +5,7 @@ import pytest from ruamel.yaml import YAML +from exasol.toolbox.util.skill_eval import PackagedSkillEvalCases from exasol.toolbox.util.skills import ( PTB_SKILL_NAME, get_packaged_skill_names, @@ -110,34 +111,40 @@ def test_ptb_skill_frontmatter_is_complete(): @pytest.mark.parametrize("skill_name", _skills_with_eval_cases()) class TestPackagedSkillEvalCases: + @pytest.fixture(scope="module") + def eval_cases_by_skill(self): + # Parse each packaged artifact once so all checks use the same model. + return { + skill_name: PackagedSkillEvalCases.model_validate( + _load_eval_cases(skill_name) + ) + for skill_name in _skills_with_eval_cases() + } + @pytest.fixture - def eval_cases(self, skill_name): - # Load the packaged artifact once so all checks inspect the same data. - return _load_eval_cases(skill_name) + def eval_cases(self, eval_cases_by_skill, skill_name): + return eval_cases_by_skill[skill_name] def test_has_expected_metadata(self, eval_cases, skill_name): - - assert eval_cases["version"] == 1 - assert eval_cases["skill"] == skill_name - assert isinstance(eval_cases["cases"], list) - assert eval_cases["cases"] + assert eval_cases.version == 1 + assert eval_cases.skill == skill_name + assert eval_cases.cases def test_cases_have_required_fields(self, eval_cases): - for case in eval_cases["cases"]: - assert case["id"].strip() - assert case["category"].strip() - assert case["prompt"].strip() + for case in eval_cases.cases: + assert case.id + assert case.category + assert case.prompt def test_cases_have_response_constraints(self, eval_cases): - for case in eval_cases["cases"]: - expected = case["expected"] - assert expected["must_include"] - assert expected["must_not_include"] - assert all(value.strip() for value in expected["must_include"]) - assert all(value.strip() for value in expected["must_not_include"]) + for case in eval_cases.cases: + assert case.expected.must_include + assert case.expected.must_not_include + assert all(value.strip() for value in case.expected.must_include) + assert all(value.strip() for value in case.expected.must_not_include) def test_case_ids_are_unique(self, eval_cases): - ids = [case["id"] for case in eval_cases["cases"]] + ids = [case.id for case in eval_cases.cases] assert len(ids) == len(set(ids)) From 0d6c5c2dd5fc3dd1ee13ff631136ee6c247dd360 Mon Sep 17 00:00:00 2001 From: jana-selva Date: Wed, 7 Oct 2026 14:16:14 +0530 Subject: [PATCH 10/11] format fix --- exasol/toolbox/util/skill_eval.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/exasol/toolbox/util/skill_eval.py b/exasol/toolbox/util/skill_eval.py index 2cbebb5f6..06ac03ac4 100644 --- a/exasol/toolbox/util/skill_eval.py +++ b/exasol/toolbox/util/skill_eval.py @@ -1,9 +1,15 @@ """Models for validating packaged agent-skill evaluation cases.""" -from typing import Annotated, Literal - -from pydantic import BaseModel, ConfigDict, Field - +from typing import ( + Annotated, + Literal, +) + +from pydantic import ( + BaseModel, + ConfigDict, + Field, +) NonBlankString = Annotated[str, Field(pattern=r"\S")] From d5098b9bfb0acc688701bf625052bf71b03c9988 Mon Sep 17 00:00:00 2001 From: jana-selva Date: Wed, 7 Oct 2026 14:22:55 +0530 Subject: [PATCH 11/11] Test project check session --- test/unit/nox/tasks_test.py | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 test/unit/nox/tasks_test.py diff --git a/test/unit/nox/tasks_test.py b/test/unit/nox/tasks_test.py new file mode 100644 index 000000000..800fe9c88 --- /dev/null +++ b/test/unit/nox/tasks_test.py @@ -0,0 +1,33 @@ +from unittest.mock import Mock + +from exasol.toolbox.nox import tasks +from exasol.toolbox.nox._shared import Mode + + +def test_project_check_runs_all_project_checks(monkeypatch, nox_session, tmp_path): + config = Mock(root_path=tmp_path) + context = {"coverage": True, "fwd-args": []} + python_files = ["src/example.py"] + integration_context = Mock(return_value=context) + get_python_files = Mock(return_value=python_files) + code_format = Mock() + pylint = Mock() + type_check = Mock() + coverage = Mock() + + monkeypatch.setattr(tasks, "PROJECT_CONFIG", config) + monkeypatch.setattr(tasks, "_integration_test_context", integration_context) + monkeypatch.setattr(tasks, "get_filtered_python_files", get_python_files) + monkeypatch.setattr(tasks, "_code_format", code_format) + monkeypatch.setattr(tasks, "_pylint", pylint) + monkeypatch.setattr(tasks, "_type_check", type_check) + monkeypatch.setattr(tasks, "_coverage", coverage) + + tasks.check(nox_session) + + integration_context.assert_called_once_with(nox_session, coverage=True) + get_python_files.assert_called_once_with(tmp_path) + code_format.assert_called_once_with(nox_session, Mode.Check, python_files) + pylint.assert_called_once_with(nox_session, python_files) + type_check.assert_called_once_with(nox_session, python_files) + coverage.assert_called_once_with(nox_session, config, context)