From 137bc62afb05461a195fd05331e76e497260ee90 Mon Sep 17 00:00:00 2001 From: Stefan Broenner Date: Mon, 13 Jul 2026 21:07:52 +0200 Subject: [PATCH] Clean up review findings: remove dead clarification code, fix stale version, honest max_turns docs - Remove unused execution/clarification.py (check_clarification had zero callers and was never wired into the Copilot path) plus its README claims. - Correct CopilotEval max_turns docstrings: the runner enforces timeout_s; max_turns is advisory (not hard-enforced mid-run) and caps subagent turns. - Bump pyproject version 0.5.7 -> 0.6.14 and classifier Alpha -> Beta. - Roll CHANGELOG [Unreleased] into a dated [0.6.13] section; add new entries. - git-ignore the .pytest-execution-servers/ test scratch directory. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: afaf3899-ebf3-4aef-930d-989f2eadaeb4 --- .gitignore | 3 + CHANGELOG.md | 15 ++++ README.md | 3 +- pyproject.toml | 4 +- src/pytest_skill_engineering/copilot/eval.py | 15 ++-- src/pytest_skill_engineering/copilot/judge.py | 3 +- .../execution/clarification.py | 88 ------------------- uv.lock | 2 +- 8 files changed, 33 insertions(+), 100 deletions(-) delete mode 100644 src/pytest_skill_engineering/execution/clarification.py diff --git a/.gitignore b/.gitignore index 2120279..c03bb38 100755 --- a/.gitignore +++ b/.gitignore @@ -69,3 +69,6 @@ site/ .squad/sessions/ # Squad: SubSquad activation file (local to this machine) .squad-workstream + +# pytest-skill-engineering test scratch (some server tests create a cwd dir) +.pytest-execution-servers/ diff --git a/CHANGELOG.md b/CHANGELOG.md index f1ec695..751fd8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,21 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## [Unreleased] +### Changed + +- **Docs now match the shipped behaviour of `max_turns`** — corrected `CopilotEval` docstrings that claimed the runner "enforces turn limits externally". The runner enforces `timeout_s` as the hard wall-clock limit; `max_turns` is advisory for the top-level session (not hard-enforced mid-run) and is used only to cap subagent turns. +- **`pyproject.toml` version corrected** — bumped from a stale `0.5.7` to track the real release line, and moved the Trove classifier from `Development Status :: 3 - Alpha` to `4 - Beta`. + +### Removed + +- **Dead clarification-detection code** — `execution/clarification.py` (`check_clarification`) had zero callers anywhere in the codebase and was never wired into the Copilot execution path, yet the feature was advertised in the README. Removed the unused module and the corresponding README claims to keep docs honest (per the project's no-dead-code policy). + +### Fixed + +- **`.pytest-execution-servers/` scratch directory** — some server tests created this directory in the repo root; it is now git-ignored. + +## [0.6.13] - 2026-07-13 + ### Added - **Agent Skills spec compliance** — Full support for `agentskills.io` compatibility metadata, `allowed-tools`, `scripts`, and `assets` fields in `SKILL.md` diff --git a/README.md b/README.md index b0c70a4..5d67d6f 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,7 @@ pytest-skill-engineering validates the **full skill engineering stack** that shi - **MCP Prompt Templates** — Do server-side templates produce the right behavior? - **CLI Tools** — Can Copilot use command-line interfaces effectively? -Plus **A/B testing**, **multi-turn sessions**, **clarification detection**, and **AI-powered reports** that tell you exactly what to fix. +Plus **A/B testing**, **multi-turn sessions**, and **AI-powered reports** that tell you exactly what to fix. ## How It Works @@ -115,7 +115,6 @@ You can also use Azure OpenAI or other providers if you prefer — see [Configur - **A/B Testing** — Compare instructions, skills, custom agent versions, or tool configurations - **Eval Leaderboard** — Auto-ranked by pass rate and cost - **Multi-Turn Sessions** — Test conversations that build on context -- **Clarification Detection** — Catch agents that ask questions instead of acting - **LLM Assertions** — Semantic checks with `llm_assert`, multi-dimension scoring with `llm_score`, image evaluation with `llm_assert_image` - **AI-Powered Reports** — Actionable feedback on tool descriptions, prompts, and costs - **Cost Tracking** — Copilot premium request tracking + USD estimation via `pricing.toml` diff --git a/pyproject.toml b/pyproject.toml index aebffa3..b115529 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "pytest-skill-engineering" -version = "0.5.7" +version = "0.6.14" description = "The testing framework for skill engineering. Test tool descriptions, prompt templates, agent skills, and custom agents with real LLMs. AI analyzes results and tells you what to fix." readme = "README.md" license = { text = "MIT" } @@ -14,7 +14,7 @@ authors = [ ] keywords = ["pytest", "ai", "llm", "mcp", "testing", "agents", "skill-engineering", "skills", "custom-agents"] classifiers = [ - "Development Status :: 3 - Alpha", + "Development Status :: 4 - Beta", "Framework :: Pytest", "Intended Audience :: Developers", "License :: OSI Approved :: MIT License", diff --git a/src/pytest_skill_engineering/copilot/eval.py b/src/pytest_skill_engineering/copilot/eval.py index b31240e..ad1d700 100644 --- a/src/pytest_skill_engineering/copilot/eval.py +++ b/src/pytest_skill_engineering/copilot/eval.py @@ -68,8 +68,10 @@ class CopilotEval: ``build_session_config()`` maps them to the SDK's actual ``system_message`` TypedDict. - The SDK's ``SessionConfig`` has no ``maxTurns`` field — turn limits - are enforced externally by the runner via ``timeout_s``. + The SDK's ``SessionConfig`` has no ``maxTurns`` field. The runner + enforces a hard wall-clock limit via ``timeout_s``; ``max_turns`` is + advisory for the top-level session (it is *not* hard-enforced mid-run) + and is used when configuring subagent turn caps. Example: # Minimal @@ -111,7 +113,9 @@ class CopilotEval: allowed_tools: list[str] | None = None # Allowlist (None = all) excluded_tools: list[str] | None = None # Blocklist - # Limits — enforced by the runner, NOT part of SDK SessionConfig + # Limits — the runner enforces timeout_s (hard wall-clock limit). + # max_turns is advisory for the top-level session (not hard-enforced + # mid-run) and is used to cap subagent turns. max_turns: int = 25 timeout_s: float = 300.0 @@ -168,8 +172,9 @@ def build_session_config(self) -> dict[str, Any]: skill_directories → skill_directories disabled_skills → disabled_skills - Note: ``max_turns`` is NOT part of ``SessionConfig`` — the runner - enforces turn limits externally. + Note: ``max_turns`` is NOT part of ``SessionConfig``. The runner + enforces ``timeout_s`` as the hard limit; ``max_turns`` is advisory + (not hard-enforced mid-run) and used only to cap subagent turns. """ config: dict[str, Any] = {} diff --git a/src/pytest_skill_engineering/copilot/judge.py b/src/pytest_skill_engineering/copilot/judge.py index 14fe346..ee2451d 100644 --- a/src/pytest_skill_engineering/copilot/judge.py +++ b/src/pytest_skill_engineering/copilot/judge.py @@ -1,8 +1,7 @@ """Shared Copilot SDK judge utility for LLM-as-judge evaluations. Provides a common interface for calling the Copilot SDK with judge prompts -and parsing responses. Used by llm_assert, llm_score, clarification detection, -and insights generation. +and parsing responses. Used by llm_assert, llm_score, and insights generation. """ from __future__ import annotations diff --git a/src/pytest_skill_engineering/execution/clarification.py b/src/pytest_skill_engineering/execution/clarification.py deleted file mode 100644 index e3873c9..0000000 --- a/src/pytest_skill_engineering/execution/clarification.py +++ /dev/null @@ -1,88 +0,0 @@ -"""Clarification detection using Copilot SDK as LLM-as-judge. - -Detects when an agent asks for user input instead of executing the requested task. -Uses the Copilot SDK to make a semantic evaluation of the response. -""" - -from __future__ import annotations - -import asyncio -import logging - -_logger = logging.getLogger(__name__) - -CLARIFICATION_RUBRIC = ( - "The AI assistant is asking for user input, confirmation, or clarification " - "BEFORE completing the requested task. " - "This includes asking 'Would you like me to...', 'Should I proceed...', " - "'Do you want me to...', 'Which would you prefer?', requesting confirmation " - "before acting, or asking for missing information. " - "The response FAILS this rubric (is NOT asking for clarification) if it: " - "provides the requested information directly, " - "uses past tense to describe completed actions, " - "starts with 'Done!' or 'Complete' or 'Successfully', " - "or ends with 'Let me know if...' AFTER describing completed work." -) - - -async def check_clarification( - response_text: str, - *, - judge_model: str, - timeout_seconds: float = 10.0, -) -> bool: - """Check if an agent response is asking for clarification. - - Uses the Copilot SDK as an LLM judge to semantically classify the response. - Fails open (returns False) on any error, so detection never breaks test execution. - - Args: - response_text: The agent's final response text to classify. - judge_model: Model string (e.g. "gpt-5.4-mini", "claude-sonnet-4"). - timeout_seconds: Timeout for the judge LLM call. - - Returns: - True if the response is asking for clarification, False otherwise. - """ - if not response_text or not response_text.strip(): - return False - - try: - from pytest_skill_engineering.copilot.judge import copilot_judge # noqa: PLC0415 - - async with asyncio.timeout(timeout_seconds): - prompt = ( - f"You are a judge. Determine if the following AI assistant response " - f"is asking for user input or clarification BEFORE completing the task.\n\n" - f"Rubric: {CLARIFICATION_RUBRIC}\n\n" - f"Response to evaluate:\n---\n{response_text}\n---\n\n" - f"Respond with ONLY 'PASS' if the assistant IS asking for clarification, " - f"or 'FAIL' if the assistant is NOT asking for clarification (completed the task). " - f"Include a brief reason on the second line." - ) - - # Strip model prefix if present - model = judge_model - if "/" in model: - model = model.split("/", 1)[1] - - response = await copilot_judge(prompt, model=model, timeout_seconds=timeout_seconds) - - # Parse response: PASS = asking for clarification, FAIL = not asking - verdict = response.strip().split("\n")[0].upper() - is_clarification = "PASS" in verdict and "FAIL" not in verdict - - if is_clarification: - _logger.info( - "Clarification detected in response: %s", - response_text[:100], - ) - - return is_clarification - - except TimeoutError: - _logger.debug("Clarification judge timed out after %ss", timeout_seconds) - return False - except Exception: - _logger.debug("Clarification judge failed, skipping detection", exc_info=True) - return False diff --git a/uv.lock b/uv.lock index 9955b5e..5382821 100644 --- a/uv.lock +++ b/uv.lock @@ -1324,7 +1324,7 @@ wheels = [ [[package]] name = "pytest-skill-engineering" -version = "0.5.7" +version = "0.6.14" source = { editable = "." } dependencies = [ { name = "github-copilot-sdk" },