From a0db439c9f2a4f1a44191df03029fd120d5b5246 Mon Sep 17 00:00:00 2001 From: sbroenne Date: Thu, 1 Oct 2026 20:19:19 +0200 Subject: [PATCH 1/2] Fix review findings and prepare 1.0.3 patch release Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/workflows/docs.yml | 4 +- CHANGELOG.md | 27 ++ docs/contributing/evidence-structure.md | 9 +- docs/contributing/releasing.md | 4 + docs/getting-started/custom-agents.md | 9 + docs/getting-started/iterations.md | 7 + docs/getting-started/skills.md | 9 +- docs/how-to/cost-estimation.md | 7 + docs/how-to/image-assertions.md | 6 + docs/how-to/inspect-evidence.md | 9 +- docs/how-to/test-cli-tools.md | 5 + docs/how-to/test-plugins.md | 4 + docs/reference/result.md | 42 +- .../src/skill_dogfood/checkout_workflow.py | 1 + .../src/skill_dogfood/workflow.py | 1 + examples/skill-dogfood/uv.lock | 2 +- pyproject.toml | 2 +- .../copilot/config.py | 77 +++- .../copilot/contracts.py | 13 +- src/pytest_skill_engineering/copilot/eval.py | 43 +- .../copilot/events.py | 73 +++- .../copilot/fixtures.py | 86 ++-- .../copilot/personas.py | 48 ++- .../copilot/result.py | 37 +- .../copilot/runner.py | 7 +- src/pytest_skill_engineering/core/evals.py | 39 +- src/pytest_skill_engineering/core/plugin.py | 9 +- src/pytest_skill_engineering/core/result.py | 44 +- .../core/serialization.py | 389 +++++------------- .../execution/cost.py | 5 + .../execution/servers.py | 18 +- src/pytest_skill_engineering/plugin.py | 86 ++-- .../reporting/collector.py | 14 +- tests/contracts/test_native_evidence.py | 1 + tests/contracts/test_review_regressions.py | 365 ++++++++++++++++ tests/contracts/test_usage_evidence.py | 18 +- .../copilot/test_12_custom_agents.py | 16 + tests/integration/copilot/test_13_plugins.py | 55 ++- tests/integration/copilot/test_events.py | 61 +++ tests/unit/test_benchmark_controls.py | 2 + tests/unit/test_copilot_agent.py | 2 +- tests/unit/test_copilot_personas.py | 20 +- tests/unit/test_plugin_hooks.py | 71 ++++ tests/unit/test_release_invariants.py | 6 +- uv.lock | 2 +- 45 files changed, 1267 insertions(+), 488 deletions(-) create mode 100644 tests/contracts/test_review_regressions.py diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 801d6dd..7d94a5b 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -20,13 +20,13 @@ on: workflow_call: inputs: version: - description: "Version tag (e.g. v1.0.2) to validate and build from" + description: "Version tag (e.g. v1.0.3) to validate and build from" required: false type: string workflow_dispatch: inputs: version: - description: "Version tag (e.g. v1.0.2) to validate and build from" + description: "Version tag (e.g. v1.0.3) to validate and build from" required: false type: string diff --git a/CHANGELOG.md b/CHANGELOG.md index 30559dc..b5cdc93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,33 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). frozen guidance exists only as a historical experiment fixture. Evaluating consumer-provided skills remains supported. +## [1.0.3] - 2026-10-01 + +### Fixed + +- Repeated pytest cases preserve grouped parameters and custom case IDs. + Saved outcomes include setup and cleanup failures, along with verification + recorded during cleanup. +- Plugin and project custom agents preserve declared names, models, tools, + reasoning effort, skills, inference settings, and MCP configuration. + Duplicate declared names are rejected consistently. +- Skill reference tools expose enabled skills only and retain nested relative + Markdown paths without silently replacing duplicate references. +- Framework-owned child executions retain separate results and descendants. + Overall token, premium-request, cost, and evidence completeness totals include + child work without counting it twice. +- Tool completion evidence preserves images, including multiple images and + explicit capture errors for malformed or conflicting payloads. +- Direct CLI execution preserves Windows paths, grouping quotes, spaces, and + empty arguments. +- Saved configuration records the actual prepared runtime settings instead of + reconstructing them from the original eval. A/B roles remain separate. +- Missing-price warnings include every captured usage model. Unknown model + identities remain null instead of receiving an invented default. +- Native schema-4.0 loading requires every current producer field and preserves + explicit nulls, empty values, and image bytes. Regenerate older incomplete + reports through their producer. + ## [1.0.2] - 2026-10-01 ### Fixed diff --git a/docs/contributing/evidence-structure.md b/docs/contributing/evidence-structure.md index dcfa011..2aa9dff 100644 --- a/docs/contributing/evidence-structure.md +++ b/docs/contributing/evidence-structure.md @@ -18,6 +18,10 @@ Schema 4.0 saves test IDs, outcomes, assertion errors, verification properties, execution configuration, turns, tool arguments and outputs, completion status, capture errors, request audits, stop reasons, usage, and missing-pricing models. Private SDK/session fields are not serialized. +Prepared session settings reflect overrides and persona additions. A missing +snapshot or unknown model is explicit `null`. Framework-owned child results +remain nested under invocation IDs, with inclusive top-level accounting; tool +image bytes and any additional images round-trip without loss. `eval_result.skill_discovery` saves actual public SDK availability separately from requested configuration and observed reads. It preserves metadata, loading @@ -28,11 +32,14 @@ availability from supplied paths or patch saved JSON. Preserve missing versus empty output and nullable usage. Do not replace unsupported property values with strings or synthesize uncaptured successful assertions. -The loader rejects old schemas; there is no compatibility reader. +The loader rejects old schemas and missing current-schema fields at every record +level; there is no compatibility reader or reconstruction from default values. Each captured run retains its pytest outcome. Multiple executions in one test share that outcome, so their entries are not independently graded task verdicts. Preserve side-specific properties and captured comparison roles. +Finalize outcomes and verification properties after fixture cleanup; setup and +cleanup errors with captured executions must not be saved as passes. ## Validation diff --git a/docs/contributing/releasing.md b/docs/contributing/releasing.md index 9a75cea..19cc408 100644 --- a/docs/contributing/releasing.md +++ b/docs/contributing/releasing.md @@ -85,6 +85,10 @@ Check the [breaking-change guide](../migration.md), complete current documentati schema-4.0 evidence contracts, package version metadata, and the installed wheel before publishing. There is no companion skill to install or publish. Reading saved evidence must work without Copilot. +Confirm strict current-field loading, null model identity, image and child-result +round trips, and outcomes finalized after fixture cleanup. A/B roles are saved +separately from prepared runtime settings. Regenerate evidence missing required +schema-4.0 fields through its producer; do not patch old reports. Verify the quickstart and the [historical customer workflow sample](https://github.com/sbroenne/pytest-skill-engineering/tree/main/examples/skill-dogfood) diff --git a/docs/getting-started/custom-agents.md b/docs/getting-started/custom-agents.md index 5bb329b..d85a825 100644 --- a/docs/getting-started/custom-agents.md +++ b/docs/getting-started/custom-agents.md @@ -9,6 +9,12 @@ A **custom agent** is a `.agent.md` definition file that you load with `load_cus It is **not** inherently a subagent. It only becomes a **subagent** when Copilot dispatches work to it at runtime. +Standalone, plugin, and project loaders preserve declared agent names, model, +tools, reasoning effort, inference settings, skills, and `mcp-servers` settings. +When no name is declared, the filename supplies it. Directory include/exclude +filters and per-agent overrides use that resolved name. Duplicate declared names +are errors; an explicit empty tool list remains empty. + ## Load one custom agent ```python @@ -37,6 +43,9 @@ async def test_dispatches_to_reviewer(copilot_eval): ``` `result.subagent_invocations` reports runtime subagent events such as `selected`, `started`, and `completed`. +Framework-owned invocations also retain a separate child `result`. Inspect its +tools, usage, and capture errors without mixing its conversation into the parent. +See [child evidence and accounting](../reference/result.md#child-execution-evidence). Each nested invocation receives the remaining custom agent registry without its own definition. That registry shrinks along the dispatch path, so a custom agent diff --git a/docs/getting-started/iterations.md b/docs/getting-started/iterations.md index 10d2cc5..5d58d7e 100644 --- a/docs/getting-started/iterations.md +++ b/docs/getting-started/iterations.md @@ -15,6 +15,13 @@ execution cost. Authorize the scope before running a large case matrix. Each repetition has its own pytest outcome. Native JSON preserves that execution, its iteration number, configuration, usage, cost estimate, duration, and recorded checks. There is no grouped dashboard verdict or automatic explanation. +Grouped and stacked pytest parameters retain their user-supplied IDs. Only the +framework's repetition axis is removed from the saved case name; the iteration +number remains a separate field. + +Saved outcomes are finalized after fixture cleanup. A cleanup failure changes +the captured execution's outcome to failed and retains cleanup-recorded checks, +even when the test body passed. There is no AI flakiness explanation. Inspect the individual traces and source to distinguish intermittent interface behavior from unstable fixtures or diff --git a/docs/getting-started/skills.md b/docs/getting-started/skills.md index 0f6c146..0e5cec9 100644 --- a/docs/getting-started/skills.md +++ b/docs/getting-started/skills.md @@ -47,8 +47,9 @@ For detailed budgeting advice, use the reference document. References are documents the agent can look up **on demand** rather than having them always in context. Copilot loads skills natively and can read their -`references/` files using its file tools. The plugin does not inject synthetic -reference tools. +`references/` files using its file tools. The supplied personas also expose +`list_skill_references` and `read_skill_reference` for validated enabled +references. `client_mode="empty"` does not inject persona tools. Reference documents may be organized in subdirectories, for example `references/commands/read.md`. Link to them directly from `SKILL.md` using paths @@ -59,6 +60,10 @@ The package validates all reference files recursively as nonempty UTF-8 Markdown `Skill.references` uses paths relative to `references/`, with `/` separators on every platform: `commands/read.md`. Flat names such as `budgeting-guide.md` are unchanged, and equal basenames in different folders remain distinct. +Reference tools use these same relative paths. Disabled skills are not +advertised or readable through the reference tools. This does not block ordinary +filesystem access by other tools. Duplicate relative reference paths across +enabled skills are rejected rather than silently replacing a document. Links are accepted only when their resolved targets stay within `references/`; the `references/` directory itself must resolve within the skill directory. Broken links, directory cycles, unreadable entries, and non-file entries are diff --git a/docs/how-to/cost-estimation.md b/docs/how-to/cost-estimation.md index 8893116..e6c50fb 100644 --- a/docs/how-to/cost-estimation.md +++ b/docs/how-to/cost-estimation.md @@ -20,9 +20,16 @@ The generated starter writes explicit rates without replacing existing ones. A missing model rate produces a placeholder zero estimate and a missing-pricing warning. The suite saves `models_without_pricing` in native JSON evidence. +Warnings cover every captured usage model, including earlier models and +framework-owned child sessions, not just the test's final display model. +They are calculated for the current suite and current pricing file. **This is not measured free usage.** Do not choose a supposedly cheapest configuration from unavailable prices. Missing SDK usage values remain `None`, not known zero. Estimates from available input/output counts are not complete billing evidence; unknown cache reads receive no cache discount. See [result evidence](../reference/result.md). + +Parent cost and premium-request totals include framework-owned child work once; +the saved child records remain separately inspectable. Do not sum inclusive +parent and child totals together. diff --git a/docs/how-to/image-assertions.md b/docs/how-to/image-assertions.md index a782add..14ac425 100644 --- a/docs/how-to/image-assertions.md +++ b/docs/how-to/image-assertions.md @@ -10,6 +10,12 @@ Tool-returned images are captured in results and preserved in native JSON. Inspect `image_content` (bytes) and `image_media_type` on calls returned by `result.tool_calls_for(...)`. +The first image remains in those fields. All later images from the same +completion are in `call.additional_images`, as `ImageContent` objects with +`data` and `media_type`. Saved `EvalResult.tool_images_for(name)` returns every +image for that tool, in order. SDK content blocks and binary image results are +captured; malformed image data produces an explicit capture error. + Semantic image judging is not supported. Use ordinary pytest assertions for image presence and metadata. Your coding agent can inspect the captured images; the framework does not automatically interpret them. diff --git a/docs/how-to/inspect-evidence.md b/docs/how-to/inspect-evidence.md index 896deb4..2a7234d 100644 --- a/docs/how-to/inspect-evidence.md +++ b/docs/how-to/inspect-evidence.md @@ -48,7 +48,9 @@ saved under `tests[].properties`; recording a property alone does not determine the outcome. Record the observed values before asserting, so failures retain the check's evidence. Use JSON-compatible values and never record secrets. -Setup failures, ordinary tests without eval execution, and opt-out skips remain +Setup failures with a captured eval and fixture-cleanup failures are reflected +in the saved execution outcome. Cleanup-recorded properties are included too. +Ordinary tests, setup failures without eval execution, and opt-out skips remain in pytest and JUnit output. Their absence from execution evidence is not a pass. ## Investigate with your coding agent @@ -74,6 +76,8 @@ No companion skill is required. `ab_run` saves both captured configurations and comparison roles without changing runtime names. Both entries share the enclosing pytest outcome and properties. +The role is stored in `eval_result.comparison_role`, separately from prepared +session settings. Record `baseline_verification` and `treatment_verification` separately. A failed combined assertion does not prove both sides failed. @@ -96,5 +100,8 @@ for case in evidence.tests: ``` Reading schema-4.0 evidence is local and needs no Copilot process or credentials. +Every current producer field is required, including nested child and tool records. +Present `null`, empty collections, empty output, and known zero remain distinct +from absent fields. Unknown models remain `null`, not a guessed model name. Regenerate older evidence by running its producer with framework 1.x; never hand-edit JSON or invent missing fields. diff --git a/docs/how-to/test-cli-tools.md b/docs/how-to/test-cli-tools.md index c375fac..700b9af 100644 --- a/docs/how-to/test-cli-tools.md +++ b/docs/how-to/test-cli-tools.md @@ -43,6 +43,11 @@ git_server = CLIServer( ## Configuration options +With `shell="none"`, both `command` and tool `args` are split into direct process +arguments, without shell expansion. On Windows, backslashes in paths are +preserved; quote paths containing spaces. Grouping quotes are removed, and +explicit empty quoted arguments remain empty arguments. + ```python CLIServer( command="git", diff --git a/docs/how-to/test-plugins.md b/docs/how-to/test-plugins.md index 09136bd..a78f473 100644 --- a/docs/how-to/test-plugins.md +++ b/docs/how-to/test-plugins.md @@ -105,6 +105,10 @@ async def test_plugin_tool_usage(copilot_eval): ``` `CopilotEval.from_plugin()` loads instructions, skills, and custom agents from the directory. The Copilot SDK handles model selection and tool access. +Custom-agent definitions retain their declared names and supported SDK settings, +including model, tools, skills, reasoning effort, and MCP servers, just as they do +when loaded through the project factories. Missing names use the filename; +duplicate resolved names are errors. ### Overriding Plugin Settings diff --git a/docs/reference/result.md b/docs/reference/result.md index cf7825c..5037ef9 100644 --- a/docs/reference/result.md +++ b/docs/reference/result.md @@ -13,8 +13,9 @@ description: "CopilotResult reference: tool calls, responses, token usage, files | `success` | Whether the eval completed successfully | | `error` | Error message when `success` is false | | `turns` | Normalized conversation turns | -| `usage` | Per-turn token usage entries | -| `model_used` | Actual Copilot model used | +| `usage` | Per-turn token usage entries for this session | +| `all_usage` | Captured usage across this session and framework-owned descendants | +| `model_used` | Actual Copilot model used, or `None` when unknown | | `total_premium_requests` | Premium-request accounting | | `subagent_invocations` | Runtime subagent events from custom agent dispatch | | `permission_requested` | Whether Copilot requested permissions | @@ -25,6 +26,7 @@ description: "CopilotResult reference: tool calls, responses, token usage, files | `tool_calls_admitted` | Calls admitted by the framework after caller guards; not proof of execution or application success | | `request_audit` | Actual outgoing request records when `audit_requests=True` | | `skill_discovery` | Actual pre-execution SDK skill metadata and diagnostics for explicit directories; `None` means no discovery check was attempted | +| `configuration` | Safe snapshot of the prepared session settings; `None` means the snapshot was not captured | `success` describes session execution, not whether the application did the right thing. A tool can fail and the session can recover. Check application state with @@ -99,7 +101,23 @@ result.tool_was_called_with("transfer", amount=500.0) ``` Tool-returned images are available on individual tool calls as `image_content` -(bytes) and `image_media_type`. See [Tool-returned images](../how-to/image-assertions.md). +(bytes) and `image_media_type`. Further images from the same call are retained in +`additional_images`, in order. See [Tool-returned images](../how-to/image-assertions.md). + +## Child execution evidence + +Each `subagent_invocations` entry retains its `invocation_id`, name, status, and +duration. For a framework-owned child session, `invocation.result` contains that +child's separate `CopilotResult`, including its own turns, tools, usage, request +audit, capture errors, prepared settings, and further children. Native SDK events +without an independently captured child result leave `result=None`; the framework +does not invent a child trace. + +`execution_results` walks this session and its framework-owned descendants. +Parent `turns`, `all_tool_calls`, and `usage` remain session-local. Token totals, +premium-request totals, and saved cost estimates include descendant work once. +Do not add already-inclusive child totals to the parent's totals again. +Parent `evidence_complete` also checks captured child evidence. ## Token helpers @@ -113,11 +131,25 @@ result.token_usage Each `usage` entry retains `model`, `input_tokens`, `output_tokens`, `cache_read_tokens`, `cache_write_tokens`, `reasoning_tokens`, `reasoning_effort`, and `duration_ms`. Missing SDK values are `None`, not zero. -Token totals are `None` when any corresponding count is unavailable (or no -usage was captured); `token_usage` omits those totals. Known zero remains zero. +Token totals are `None` when any corresponding count is unavailable, or a +captured session in the execution tree has no usage; `token_usage` omits those +totals. Known zero remains zero. Cost is an estimate from reported input/output counts, not a claim of complete billing evidence; unknown cache reads receive no cache discount in that estimate. +## Prepared settings + +`configuration` records the supplied settings after `extra_config` overrides, +persona instructions, and tool setup, rather than rebuilding them during report +conversion. Native evidence's `effective_system_prompt` is this supplied system +message; it is not a claim to capture hidden SDK instructions. If preparation +fails before the snapshot, both remain `None`. + +Server credentials, commands, arguments, environment variables, and executable +handlers are excluded. Request audit remains the separate source of evidence +about actual outgoing requests. A/B comparison roles are report metadata in +`eval_result.comparison_role`, not part of the runtime configuration. + ## Request audit Each `request_audit` record contains: diff --git a/examples/skill-dogfood/src/skill_dogfood/checkout_workflow.py b/examples/skill-dogfood/src/skill_dogfood/checkout_workflow.py index 06fe157..8af137a 100644 --- a/examples/skill-dogfood/src/skill_dogfood/checkout_workflow.py +++ b/examples/skill-dogfood/src/skill_dogfood/checkout_workflow.py @@ -76,6 +76,7 @@ def check_checkout_consumer( assert case.eval_result is not None result = case.eval_result assert result.success and result.evidence_complete, result.error + assert result.configuration is not None, "Prepared session settings were not captured" assert result.configuration["model"] == MODEL calls = [call for turn in result.turns for call in turn.tool_calls] assert len(calls) == 1 and calls[0].name == "checkout" diff --git a/examples/skill-dogfood/src/skill_dogfood/workflow.py b/examples/skill-dogfood/src/skill_dogfood/workflow.py index b110897..1f31900 100644 --- a/examples/skill-dogfood/src/skill_dogfood/workflow.py +++ b/examples/skill-dogfood/src/skill_dogfood/workflow.py @@ -261,6 +261,7 @@ def _check_consumer( assert case.eval_result is not None, run.stdout + run.stderr result = case.eval_result assert result.success and result.evidence_complete, result.error + assert result.configuration is not None, "Prepared session settings were not captured" assert result.configuration["model"] == MODEL calls = [call for turn in result.turns for call in turn.tool_calls] assert len(calls) == 1 and calls[0].name == "invoice" diff --git a/examples/skill-dogfood/uv.lock b/examples/skill-dogfood/uv.lock index c5ba0ee..e8008ca 100644 --- a/examples/skill-dogfood/uv.lock +++ b/examples/skill-dogfood/uv.lock @@ -626,7 +626,7 @@ wheels = [ [[package]] name = "pytest-skill-engineering" -version = "1.0.2" +version = "1.0.3" source = { editable = "../../" } dependencies = [ { name = "github-copilot-sdk" }, diff --git a/pyproject.toml b/pyproject.toml index c15851e..89b3f35 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "pytest-skill-engineering" -version = "1.0.2" +version = "1.0.3" description = "Reliable Copilot execution and evidence for coding-agent-led testing of MCP servers, CLI tools, skills, and custom agents." readme = "README.md" license = { text = "MIT" } diff --git a/src/pytest_skill_engineering/copilot/config.py b/src/pytest_skill_engineering/copilot/config.py index 0ac84a4..dc56c9a 100644 --- a/src/pytest_skill_engineering/copilot/config.py +++ b/src/pytest_skill_engineering/copilot/config.py @@ -6,11 +6,86 @@ from __future__ import annotations +from copy import deepcopy from pathlib import Path -from typing import Any +from typing import TYPE_CHECKING, Any from pytest_skill_engineering.core.plugin import _read_json, _validate_mcp_servers +if TYPE_CHECKING: + from pytest_skill_engineering.copilot.contracts import CopilotEvalConfig + + +def snapshot_session_configuration( + agent: CopilotEvalConfig, session_config: dict[str, Any] +) -> dict[str, Any]: + """Capture prepared settings without handlers, credentials, or server environments.""" + system = session_config.get("system_message") or {} + snapshot = { + "name": agent.name, + "model": session_config.get("model"), + "instructions": system.get("content"), + "system_message_mode": system.get("mode"), + "reasoning_effort": session_config.get("reasoning_effort"), + "client_mode": agent.client_mode, + "image_detail": agent.image_detail, + "audit_requests": agent.audit_requests, + "max_tool_calls": agent.max_tool_calls, + "max_turns": agent.max_turns, + "timeout_s": agent.timeout_s, + "auto_confirm": agent.auto_confirm, + "allowed_tools": session_config.get("available_tools"), + "excluded_tools": session_config.get("excluded_tools"), + "skill_directories": session_config.get("skill_directories", []), + "disabled_skills": session_config.get("disabled_skills", []), + "active_agent": session_config.get("agent"), + "working_directory": session_config.get("working_directory"), + "persona": type(agent.persona).__name__, + "tools": [ + { + "name": tool.name, + "description": tool.description, + "parameters": tool.parameters, + "metadata": tool.metadata, + } + for tool in session_config.get("tools") or [] + ], + "mcp_servers": { + name: {"type": config.get("type"), "tools": config.get("tools")} + for name, config in (session_config.get("mcp_servers") or {}).items() + }, + "custom_agents": [ + { + key: value + for key, value in custom.items() + if key + in { + "name", + "prompt", + "description", + "display_name", + "model", + "reasoning_effort", + "tools", + "infer", + "skills", + } + } + for custom in session_config.get("custom_agents") or [] + ], + } + for key in ( + "enable_config_discovery", + "enable_file_hooks", + "enable_on_demand_instruction_discovery", + "skip_custom_instructions", + "enable_managed_settings", + "enable_skills", + ): + if key in session_config: + snapshot[key] = session_config[key] + return deepcopy(snapshot) + def load_mcp_config(path: str | Path) -> dict[str, dict[str, Any]]: """Load MCP server configs from a ``.mcp.json`` file. diff --git a/src/pytest_skill_engineering/copilot/contracts.py b/src/pytest_skill_engineering/copilot/contracts.py index 014b6ea..b9dc04c 100644 --- a/src/pytest_skill_engineering/copilot/contracts.py +++ b/src/pytest_skill_engineering/copilot/contracts.py @@ -4,10 +4,13 @@ from collections.abc import Awaitable, Callable, Mapping from dataclasses import dataclass -from typing import Any, Literal, Protocol, TypeAlias +from typing import TYPE_CHECKING, Any, Literal, Protocol, TypeAlias from copilot.session import CustomAgentConfig, MCPServerConfig, ReasoningEffort, SessionHooks +if TYPE_CHECKING: + from pytest_skill_engineering.copilot.result import CopilotResult + CopilotCustomAgentConfig: TypeAlias = CustomAgentConfig CopilotMCPServerConfig: TypeAlias = MCPServerConfig CopilotReasoningEffort: TypeAlias = ReasoningEffort @@ -53,6 +56,11 @@ def record_subagent_complete(self, *, invocation_id: str, name: str) -> None: def record_subagent_failed(self, *, invocation_id: str, name: str) -> None: raise NotImplementedError + def record_subagent_result( + self, *, invocation_id: str, name: str, result: CopilotResult + ) -> None: + raise NotImplementedError + class CopilotEvalConfig(CopilotResultAgent, Protocol): """Configuration surface consumed by the runner and personas.""" @@ -141,7 +149,7 @@ def create_subagent( CopilotNestedRunner: TypeAlias = Callable[ [CopilotEvalConfig, str], - Awaitable[CopilotRunResult], + Awaitable["CopilotResult"], ] @@ -166,6 +174,7 @@ class SubagentInvocation: name: str status: SubagentStatus duration_ms: float | None = None + result: CopilotResult | None = None def declared_agent_tools( diff --git a/src/pytest_skill_engineering/copilot/eval.py b/src/pytest_skill_engineering/copilot/eval.py index 1be569c..669ca60 100644 --- a/src/pytest_skill_engineering/copilot/eval.py +++ b/src/pytest_skill_engineering/copilot/eval.py @@ -18,7 +18,6 @@ from pytest_skill_engineering.core.evals import load_custom_agent, load_instruction_file from pytest_skill_engineering.core.plugin import ( _discover_skills, - _validate_mcp_servers, _validate_skill_reference_names, ) @@ -34,30 +33,20 @@ def _parse_agent_file(path: Path) -> CopilotCustomAgentConfig: fields and uses the Markdown body as the custom agent's system prompt. Parsing and metadata errors retain the definition's file path. """ - loaded = load_custom_agent(path) - frontmatter = loaded["metadata"] - agent: CopilotCustomAgentConfig = { - "name": frontmatter.get("name", loaded["name"]), - "prompt": loaded["prompt"], - } - for key in ( - "description", - "display_name", - "infer", - "skills", - "model", - "reasoning_effort", - "tools", - ): - if key in frontmatter: - agent[key] = frontmatter[key] - if "mcp-servers" in frontmatter: - agent["mcp_servers"] = cast( - dict[str, CopilotMCPServerConfig], - _validate_mcp_servers(frontmatter["mcp-servers"], path, field="mcp-servers"), - ) + return cast(CopilotCustomAgentConfig, load_custom_agent(path)) + - return agent +def _parse_agent_files(paths: list[Path]) -> list[CopilotCustomAgentConfig]: + agents: list[CopilotCustomAgentConfig] = [] + names: set[str] = set() + for path in paths: + agent = _parse_agent_file(path) + name = require_custom_agent_name(agent) + if name in names: + raise ValueError(f"{path}: Duplicate custom agent name '{name}'") + names.add(name) + agents.append(agent) + return agents def _default_persona() -> CopilotPersona: @@ -361,8 +350,7 @@ def from_copilot_config( if agents_dir.exists(): if not agents_dir.is_dir(): raise ValueError(f"{agents_dir}: custom agents path must be a directory") - for agent_file in sorted(agents_dir.rglob("*.agent.md")): - agents.append(_parse_agent_file(agent_file)) + agents = _parse_agent_files(sorted(agents_dir.rglob("*.agent.md"))) config: dict[str, Any] = { "instructions": instructions, @@ -527,8 +515,7 @@ def from_claude_config( if not agents_dir.is_dir(): raise ValueError(f"{agents_dir}: custom agents path must be a directory") # Claude Code uses plain .md files for agents - for agent_file in sorted(agents_dir.glob("*.md")): - agents.append(_parse_agent_file(agent_file)) + agents = _parse_agent_files(sorted(agents_dir.glob("*.md"))) # 3. Discover skill directories from .claude/skills/ skills = _discover_skills(claude_dir) diff --git a/src/pytest_skill_engineering/copilot/events.py b/src/pytest_skill_engineering/copilot/events.py index e611544..afe009e 100644 --- a/src/pytest_skill_engineering/copilot/events.py +++ b/src/pytest_skill_engineering/copilot/events.py @@ -56,6 +56,8 @@ from __future__ import annotations +import base64 +import binascii import logging import time from collections import deque @@ -70,6 +72,7 @@ Turn, UsageInfo, ) +from pytest_skill_engineering.core.result import ImageContent if TYPE_CHECKING: from copilot.generated.session_events import SessionEvent @@ -162,7 +165,12 @@ def build(self) -> CopilotResult: permissions=self._permissions, model_used=self._model_used, raw_events=self._raw_events, - total_premium_requests=self._total_premium_requests, + total_premium_requests=self._total_premium_requests + + sum( + invocation.result.total_premium_requests + for invocation in resolved_subagents + if invocation.result is not None + ), ) # ── Assistant events ── @@ -342,22 +350,73 @@ def _handle_tool_execution_complete(self, event: SessionEvent) -> None: self._early_tool_completions.setdefault(call_id, []).append(event) return result_text: str | None = None + images: list[ImageContent] = [] if result_data is not None and hasattr(result_data, "content"): result_text = result_data.content elif isinstance(result_data, str): result_text = result_data elif result_data is not None: result_text = str(result_data) + text_parts: list[str] = [] + blocks = getattr(result_data, "contents", None) or [] + for block in blocks: + block_type = getattr(block, "type", None) + if block_type == "image": + try: + data = getattr(block, "data", None) + media_type = getattr(block, "mime_type", None) + if ( + not isinstance(data, str) + or not isinstance(media_type, str) + or not media_type + ): + raise ValueError("image data and media type are required") + images.append(ImageContent(base64.b64decode(data, validate=True), media_type)) + except (ValueError, binascii.Error) as exc: + self._record_contract_error(f"Tool call {call_id} has an invalid image: {exc}") + elif block_type == "text" and result_text is None: + text = getattr(block, "text", None) + if text is not None: + text_parts.append(text) + if result_text is None and text_parts: + result_text = "\n".join(text_parts) + content_images = list(images) + for block in getattr(result_data, "binary_results_for_llm", None) or []: + block_type = getattr(block, "type", None) + if getattr(block_type, "value", block_type) != "image": + continue + try: + data, media_type = block.data, block.mime_type + if not isinstance(data, str) or not isinstance(media_type, str) or not media_type: + raise ValueError("image data and media type are required") + image = ImageContent(base64.b64decode(data, validate=True), media_type) + if image not in content_images: + images.append(image) + except (ValueError, binascii.Error) as exc: + self._record_contract_error(f"Tool call {call_id} has an invalid image: {exc}") success = _get_data_field(event, "success", None) error = _stringify_tool_error(_get_data_field(event, "error", None)) if tc.completion_received: - if (tc.result, tc.success, tc.error) != (result_text, success, error): + captured_images = ( + [ImageContent(tc.image_content, tc.image_media_type or "image/png")] + if tc.image_content is not None + else [] + ) + tc.additional_images + if (tc.result, tc.success, tc.error, captured_images) != ( + result_text, + success, + error, + images, + ): self._record_contract_error(f"Conflicting completion for tool call {call_id}") return tc.completion_received = True tc.success = success tc.result = result_text tc.error = error + if images: + tc.image_content, tc.image_media_type = images[0].data, images[0].media_type + tc.additional_images = images[1:] if tc.success is None: self._record_contract_error(f"Tool call {call_id} completion is missing success") start = self._pending_tool_start_times.pop(call_id, None) @@ -382,6 +441,16 @@ def _handle_abort(self, event: SessionEvent) -> None: # ── Subagent recording (used by runSubagent tool handler) ── + def record_subagent_result( + self, *, invocation_id: str, name: str, result: CopilotResult + ) -> None: + """Link a framework-owned child without mixing its conversation into this one.""" + invocation = self._ensure_subagent_invocation(invocation_id=invocation_id, name=name) + if invocation.result is not None and invocation.result is not result: + self._record_contract_error(f"Conflicting child result for invocation {invocation_id}") + return + invocation.result = result + def record_subagent_start(self, *, invocation_id: str, name: str) -> None: """Record a subagent invocation dispatched via a tool call.""" invocation = self._ensure_subagent_invocation(invocation_id=invocation_id, name=name) diff --git a/src/pytest_skill_engineering/copilot/fixtures.py b/src/pytest_skill_engineering/copilot/fixtures.py index 2fb594f..1a40daf 100644 --- a/src/pytest_skill_engineering/copilot/fixtures.py +++ b/src/pytest_skill_engineering/copilot/fixtures.py @@ -83,7 +83,7 @@ def _convert_to_aitest( """ from dataclasses import dataclass - from pytest_skill_engineering.core.result import EvalResult + from pytest_skill_engineering.core.result import EvalResult, SubagentInvocation from pytest_skill_engineering.execution.cost import estimate_cost # Estimate USD cost from captured token usage and pricing.toml. @@ -96,7 +96,7 @@ def _convert_to_aitest( usage.output_tokens, usage.cache_read_tokens or 0, ) - for usage in result.usage + for usage in result.all_usage if usage.input_tokens is not None and usage.output_tokens is not None ) @@ -108,7 +108,9 @@ def _convert_to_aitest( duration_ms=result.duration_ms, token_usage=result.token_usage, cost_usd=cost_usd, - effective_system_prompt=agent.instructions or "", + effective_system_prompt=( + result.configuration.get("instructions") if result.configuration is not None else None + ), premium_requests=result.total_premium_requests, evidence_complete=result.evidence_complete, capture_errors=list(result.capture_errors), @@ -117,49 +119,39 @@ def _convert_to_aitest( usage=list(result.usage), tool_calls_admitted=result.tool_calls_admitted, skill_discovery=deepcopy(result.skill_discovery), - configuration=deepcopy( - { - "name": agent.name, - "model": agent.model, - "instructions": agent.instructions, - "system_message_mode": agent.system_message_mode, - "reasoning_effort": agent.reasoning_effort, - "client_mode": agent.client_mode, - "image_detail": agent.image_detail, - "audit_requests": agent.audit_requests, - "max_tool_calls": agent.max_tool_calls, - "tools": [ - { - "name": tool.name, - "description": tool.description, - "parameters": tool.parameters, - "metadata": tool.metadata, - } - for tool in agent.extra_config.get("tools", []) - ], - "allowed_tools": agent.allowed_tools, - "excluded_tools": agent.excluded_tools, - "max_turns": agent.max_turns, - "timeout_s": agent.timeout_s, - "auto_confirm": agent.auto_confirm, - "mcp_servers": { - name: {"type": config.get("type"), "tools": config.get("tools")} - for name, config in agent.mcp_servers.items() - }, - "skill_directories": agent.skill_directories, - "disabled_skills": agent.disabled_skills, - "active_agent": agent.active_agent, - "persona": type(agent.persona).__name__, - } - ), + configuration=deepcopy(result.configuration), + model_used=result.model_used, + reasoning_traces=list(result.reasoning_traces), + permission_requested=result.permission_requested, + permissions=deepcopy(result.permissions), ) + for invocation in result.subagent_invocations: + child_result = None + if invocation.result is not None: + from pytest_skill_engineering.copilot.eval import CopilotEval + + child_agent = invocation.result.agent + if not isinstance(child_agent, CopilotEval): + raise ValueError(f"Child invocation {invocation.invocation_id!r} has no eval") + converted_child = _convert_to_aitest(child_agent, invocation.result) + assert converted_child is not None + child_result = converted_child[0] + aitest_result.subagent_invocations.append( + SubagentInvocation( + invocation_id=invocation.invocation_id, + name=invocation.name, + status=invocation.status, + duration_ms=invocation.duration_ms, + result=child_result, + ) + ) # Create a minimal wrapper with just the fields needed by plugin.py - @dataclass + @dataclass(slots=True) class Provider: - model: str + model: str | None - @dataclass + @dataclass(slots=True) class AgentWrapper: name: str id: str @@ -176,7 +168,15 @@ def __post_init__(self) -> None: aitest_agent = AgentWrapper( name=agent.name, id=agent.name, - provider=Provider(model=result.model_used or agent.model or "claude-haiku-4-5"), + provider=Provider( + model=result.model_used + if result.model_used is not None + else ( + result.configuration.get("model") + if result.configuration is not None + else agent.extra_config.get("model", agent.model) + ) + ), system_prompt_name=None, # CopilotEval doesn't have named prompts mcp_servers=[], # Could convert agent.mcp_servers if needed allowed_tools=agent.allowed_tools, @@ -211,7 +211,7 @@ def stash_on_item( report_name = f"{agent.name} ({comparison_role})" converted[1].name = report_name converted[1].id = report_name - converted[0].configuration["comparison_role"] = comparison_role + converted[0].comparison_role = comparison_role item._aitest_result = converted[0] # type: ignore[attr-defined] item._aitest_agent = converted[1] # type: ignore[attr-defined] runs = getattr(item, "_aitest_runs", None) diff --git a/src/pytest_skill_engineering/copilot/personas.py b/src/pytest_skill_engineering/copilot/personas.py index 6c3b1c5..b8590ba 100644 --- a/src/pytest_skill_engineering/copilot/personas.py +++ b/src/pytest_skill_engineering/copilot/personas.py @@ -437,6 +437,10 @@ async def _handler(invocation: "ToolInvocation") -> "ToolResult": ) sub_result = await nested_runner(sub_agent, prompt_text) + sub_result.agent = sub_agent + mapper.record_subagent_result( + invocation_id=invocation.tool_call_id, name=agent_slug, result=sub_result + ) if sub_result.success: mapper.record_subagent_complete( @@ -501,32 +505,26 @@ def _inject_skill_reference_tools( Only injected when at least one skill directory has reference documents. """ - if not agent.skill_directories: - return + from pytest_skill_engineering.copilot.skills import requested_skill_files + from pytest_skill_engineering.core.plugin import _validate_skill_reference_names + from pytest_skill_engineering.core.skill import Skill - # Collect all reference files across all skill directories - reference_files: dict[str, Path] = {} # filename → full path - - for skill_dir_str in agent.skill_directories: - skill_dir = Path(skill_dir_str) - - # Check if this is a skill directory (contains SKILL.md) - _EXTS = (".md", ".txt", ".json", ".yaml", ".yml") - if (skill_dir / "SKILL.md").exists(): - refs_dir = skill_dir / "references" - if refs_dir.is_dir(): - for ref_file in refs_dir.iterdir(): - if ref_file.is_file() and ref_file.suffix in _EXTS: - reference_files[ref_file.name] = ref_file - else: - # Maybe it's a parent directory containing skill subdirectories - for sub in skill_dir.iterdir(): - if sub.is_dir() and (sub / "SKILL.md").exists(): - refs_dir = sub / "references" - if refs_dir.is_dir(): - for ref_file in refs_dir.iterdir(): - if ref_file.is_file() and ref_file.suffix in _EXTS: - reference_files[ref_file.name] = ref_file + configured = ( + session_config if "skill_directories" in session_config else agent.build_session_config() + ) + directories = configured.get("skill_directories") + if not directories: + return + disabled = configured.get("disabled_skills") or [] + skills = [ + Skill.from_path(path) + for path, name in requested_skill_files(directories).items() + if name not in disabled + ] + _validate_skill_reference_names(skills) + reference_files = { + name: skill.path / "references" / name for skill in skills for name in skill.references + } if not reference_files: return diff --git a/src/pytest_skill_engineering/copilot/result.py b/src/pytest_skill_engineering/copilot/result.py index 9748dd4..6d9a0a3 100644 --- a/src/pytest_skill_engineering/copilot/result.py +++ b/src/pytest_skill_engineering/copilot/result.py @@ -75,11 +75,38 @@ class CopilotResult: stop_reason: StopReason | None = None tool_calls_admitted: int = 0 skill_discovery: SkillDiscovery | None = None + configuration: dict[str, Any] | None = None @property def evidence_complete(self) -> bool: """Whether every observed call has a complete, correlated record.""" - return not self.capture_errors and all(c.evidence_complete for c in self.all_tool_calls) + return ( + not self.capture_errors + and all(c.evidence_complete for c in self.all_tool_calls) + and all( + invocation.result.evidence_complete + for invocation in self.subagent_invocations + if invocation.result is not None + ) + ) + + @property + def execution_results(self) -> list[CopilotResult]: + """This session and framework-owned descendants, each included once.""" + return [ + self, + *( + result + for invocation in self.subagent_invocations + if invocation.result is not None + for result in invocation.result.execution_results + ), + ] + + @property + def all_usage(self) -> list[UsageInfo]: + """Captured usage across this session and its descendants.""" + return [usage for result in self.execution_results for usage in result.usage] # Back-reference to the agent that produced this result. # Set automatically by run_copilot() so the plugin hook can @@ -148,13 +175,17 @@ def tool_was_called_with(self, name: str, **expected_args: Any) -> bool: @property def total_input_tokens(self) -> int | None: """Total input tokens across all model turns.""" - counts = [u.input_tokens for u in self.usage] + if any(not result.usage for result in self.execution_results): + return None + counts = [u.input_tokens for u in self.all_usage] return sum(c for c in counts if c is not None) if counts and None not in counts else None @property def total_output_tokens(self) -> int | None: """Total output tokens across all model turns.""" - counts = [u.output_tokens for u in self.usage] + if any(not result.usage for result in self.execution_results): + return None + counts = [u.output_tokens for u in self.all_usage] return sum(c for c in counts if c is not None) if counts and None not in counts else None @property diff --git a/src/pytest_skill_engineering/copilot/runner.py b/src/pytest_skill_engineering/copilot/runner.py index c004f50..945aea6 100644 --- a/src/pytest_skill_engineering/copilot/runner.py +++ b/src/pytest_skill_engineering/copilot/runner.py @@ -16,7 +16,7 @@ import logging from pathlib import Path from tempfile import TemporaryDirectory -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any from pytest_skill_engineering.copilot.client import ( approve_all_permissions, @@ -84,6 +84,7 @@ async def run_copilot(agent: CopilotEvalConfig, prompt: str) -> CopilotResult: error: str | None = None cleanup_errors: list[str] = [] skill_discovery: SkillDiscovery | None = None + configuration: dict[str, Any] | None = None send_started = False try: session_config = agent.build_session_config() @@ -105,6 +106,9 @@ async def run_copilot(agent: CopilotEvalConfig, prompt: str) -> CopilotResult: controls.install(session_config, caller_hooks) if agent.auto_confirm and "on_permission_request" not in session_config: session_config["on_permission_request"] = approve_all_permissions + from pytest_skill_engineering.copilot.config import snapshot_session_configuration + + configuration = snapshot_session_configuration(agent, session_config) caller_event = session_config.get("on_event") @@ -237,6 +241,7 @@ async def execute() -> None: result = mapper.build() result.agent = agent result.skill_discovery = skill_discovery + result.configuration = configuration result.tool_calls_admitted = controls.admitted if result.capture_errors: result.capture_errors.extend(controls.incomplete_diagnostics(abort_phase=reason)) diff --git a/src/pytest_skill_engineering/core/evals.py b/src/pytest_skill_engineering/core/evals.py index 5202152..91d0c84 100644 --- a/src/pytest_skill_engineering/core/evals.py +++ b/src/pytest_skill_engineering/core/evals.py @@ -161,7 +161,7 @@ def load_custom_agent( Returns: Dict with keys: - - ``name`` (str): Derived from filename. + - ``name`` (str): Declared name, or derived from filename when absent. - ``prompt`` (str): Markdown body after frontmatter. - ``description`` (str): From frontmatter, empty if absent. - ``metadata`` (dict): Full parsed frontmatter dict. @@ -183,11 +183,20 @@ def load_custom_agent( raise ValueError(msg) config: dict[str, Any] = { - "name": _name_from_path(path), + "name": metadata.get("name", _name_from_path(path)), "prompt": body, "description": metadata.get("description", ""), "metadata": metadata, } + for key in ("display_name", "infer", "skills", "model", "reasoning_effort", "tools"): + if key in metadata: + config[key] = metadata[key] + if "mcp-servers" in metadata: + from pytest_skill_engineering.core.plugin import _validate_mcp_servers + + config["mcp_servers"] = _validate_mcp_servers( + metadata["mcp-servers"], path, field="mcp-servers" + ) if overrides: config.update(overrides) @@ -196,6 +205,22 @@ def load_custom_agent( if not isinstance(config[field], str) or not config[field].strip(): raise ValueError(f"{path}: '{field}' must be a non-empty string") _validate_metadata({**metadata, **(overrides or {})}, path) + if isinstance(config.get("tools"), str): + config["tools"] = config["tools"].replace(",", " ").split() + if "infer" in config and type(config["infer"]) is not bool: + raise ValueError(f"{path}: 'infer' must be a boolean") + if "skills" in config and not ( + isinstance(config["skills"], list) + and all(isinstance(skill, str) and skill.strip() for skill in config["skills"]) + ): + raise ValueError(f"{path}: 'skills' must be a list of non-empty strings") + if "reasoning_effort" in config and config["reasoning_effort"] not in ( + "low", + "medium", + "high", + "xhigh", + ): + raise ValueError(f"{path}: invalid 'reasoning_effort'") return config @@ -230,7 +255,8 @@ def load_custom_agents( agents: list[dict[str, Any]] = [] for path in sorted(directory.glob("*.agent.md")): - name = _name_from_path(path) + loaded = load_custom_agent(path) + name = loaded["name"] if include is not None and name not in include: continue @@ -238,9 +264,12 @@ def load_custom_agents( continue agent_overrides = (overrides or {}).get(name) - agents.append(load_custom_agent(path, overrides=agent_overrides)) + agent = load_custom_agent(path, overrides=agent_overrides) if agent_overrides else loaded + if any(existing["name"] == agent["name"] for existing in agents): + raise ValueError(f"{path}: Duplicate custom agent name '{agent['name']}'") + agents.append(agent) - return agents + return sorted(agents, key=lambda agent: agent["name"]) def _prompt_name_from_path(path: Path) -> str: diff --git a/src/pytest_skill_engineering/core/plugin.py b/src/pytest_skill_engineering/core/plugin.py index 1a0e331..ff1725b 100644 --- a/src/pytest_skill_engineering/core/plugin.py +++ b/src/pytest_skill_engineering/core/plugin.py @@ -305,15 +305,8 @@ def _discover_agents(plugin_dir: Path) -> list[dict[str, Any]]: agents: list[dict[str, Any]] = [] - # Load .agent.md files (VS Code / Copilot format) - for agent_file in sorted(agents_dir.glob("*.agent.md")): - agents.append(load_custom_agent(agent_file)) - - # Load plain .md files (Claude Code format) that aren't .agent.md - seen_names = {a["name"] for a in agents} + seen_names: set[str] = set() for agent_file in sorted(agents_dir.glob("*.md")): - if agent_file.name.endswith(".agent.md"): - continue agent = load_custom_agent(agent_file) if agent["name"] in seen_names: raise ValueError(f"{agent_file}: Duplicate custom agent name '{agent['name']}'") diff --git a/src/pytest_skill_engineering/core/result.py b/src/pytest_skill_engineering/core/result.py index 663b333..f663813 100644 --- a/src/pytest_skill_engineering/core/result.py +++ b/src/pytest_skill_engineering/core/result.py @@ -61,6 +61,7 @@ class ToolCall: duration_ms: float | None = None image_content: bytes | None = None image_media_type: str | None = None + additional_images: list[ImageContent] = field(default_factory=list) call_id: str | None = None completion_received: bool | None = None success: bool | None = None @@ -72,7 +73,10 @@ def evidence_complete(self) -> bool: self.completion_received is True and self.success is not None and ( - self.result is not None or self.error is not None or self.image_content is not None + self.result is not None + or self.error is not None + or self.image_content is not None + or bool(self.additional_images) ) ) @@ -254,9 +258,11 @@ class SubagentInvocation: assert all(s.status == "completed" for s in result.subagent_invocations) """ + invocation_id: str name: str status: str # "selected", "started", "completed", "failed" duration_ms: float | None = None + result: EvalResult | None = None def __repr__(self) -> str: return f"SubagentInvocation({self.name}, {self.status})" @@ -306,19 +312,38 @@ class EvalResult: available_tools: list[ToolInfo] = field(default_factory=list) skill_info: SkillInfo | None = None skill_discovery: SkillDiscovery | None = None - effective_system_prompt: str = "" + effective_system_prompt: str | None = None mcp_prompts: list[MCPPrompt] = field(default_factory=list) prompt_name: str | None = None custom_agent_info: CustomAgentInfo | None = None premium_requests: float = 0.0 instruction_files: list[InstructionFileInfo] = field(default_factory=list) - configuration: dict[str, Any] = field(default_factory=dict) + configuration: dict[str, Any] | None = None capture_errors: list[str] = field(default_factory=list) evidence_complete: bool | None = None request_audit: list[RequestAudit] = field(default_factory=list) stop_reason: StopReason | None = None usage: list[UsageInfo] = field(default_factory=list) tool_calls_admitted: int = 0 + model_used: str | None = None + reasoning_traces: list[str] = field(default_factory=list) + permission_requested: bool = False + permissions: list[dict[str, Any]] = field(default_factory=list) + subagent_invocations: list[SubagentInvocation] = field(default_factory=list) + comparison_role: Literal["baseline", "treatment"] | None = None + + @property + def execution_results(self) -> list[EvalResult]: + """This session and separately captured framework-owned descendants.""" + return [ + self, + *( + result + for invocation in self.subagent_invocations + if invocation.result is not None + for result in invocation.result.execution_results + ), + ] # Clarification detection clarification_stats: ClarificationStats | None = None @@ -448,11 +473,14 @@ def tool_images_for(self, name: str) -> list[ImageContent]: assert len(screenshots) > 0 assert screenshots[-1].media_type == "image/png" """ - return [ - ImageContent(data=c.image_content, media_type=c.image_media_type or "image/png") - for c in self.tool_calls_for(name) - if c.image_content is not None - ] + images: list[ImageContent] = [] + for call in self.tool_calls_for(name): + if call.image_content is not None: + images.append( + ImageContent(call.image_content, call.image_media_type or "image/png") + ) + images.extend(call.additional_images) + return images @property def asked_for_clarification(self) -> bool: diff --git a/src/pytest_skill_engineering/core/serialization.py b/src/pytest_skill_engineering/core/serialization.py index c062a02..d5588bb 100644 --- a/src/pytest_skill_engineering/core/serialization.py +++ b/src/pytest_skill_engineering/core/serialization.py @@ -1,309 +1,128 @@ -"""Serialization helpers for dataclasses to JSON-compatible dicts.""" +"""Lossless serialization and strict loading of the current native report schema.""" from __future__ import annotations import base64 +import binascii +import sys from dataclasses import fields, is_dataclass -from typing import TYPE_CHECKING, Any +from types import UnionType +from typing import TYPE_CHECKING, Any, Literal, TypeVar, Union, get_args, get_origin, get_type_hints if TYPE_CHECKING: from pytest_skill_engineering.reporting.collector import SuiteReport - -def _require_key(data: dict[str, Any], key: str, *, context: str) -> Any: - """Read a required key or raise a schema-alignment error.""" - if key not in data: - msg = f"{context} is missing required field {key!r}" - raise ValueError(msg) - return data[key] +T = TypeVar("T") def serialize_dataclass(obj: Any) -> Any: - """Convert dataclass to dict recursively, handling special types. - - Excludes private fields (prefixed with _) from serialization. - Encodes bytes fields as base64 strings. - - Uses manual field iteration instead of ``dataclasses.asdict()`` to avoid - ``copy.deepcopy`` on large private fields (e.g. ``_messages`` containing - SDK session payloads). - """ + """Convert public dataclass fields recursively, encoding bytes as base64.""" if is_dataclass(obj) and not isinstance(obj, type): - result = {} - for f in fields(obj): - if f.name.startswith("_"): - continue - v = getattr(obj, f.name) - if isinstance(v, bytes): - result[f.name] = base64.b64encode(v).decode("ascii") - else: - result[f.name] = serialize_dataclass(v) - return result - elif isinstance(obj, (list, tuple)): + return { + field.name: serialize_dataclass(getattr(obj, field.name)) + for field in fields(obj) + if not field.name.startswith("_") + } + if isinstance(obj, (list, tuple)): return [serialize_dataclass(item) for item in obj] - elif isinstance(obj, dict): - return {k: serialize_dataclass(v) for k, v in obj.items()} - elif isinstance(obj, bytes): + if isinstance(obj, dict): + return {key: serialize_dataclass(value) for key, value in obj.items()} + if isinstance(obj, bytes): return base64.b64encode(obj).decode("ascii") - else: - # For enums, strings, numbers, etc. - return obj - + return obj -def deserialize_suite_report(data: dict[str, Any]) -> SuiteReport: - """Deserialize a SuiteReport from a dict (from JSON). - Reconstructs the full dataclass hierarchy from the serialized format. - """ +def _decode_dataclass(cls: type[T], data: Any, *, path: str) -> T: from pytest_skill_engineering.copilot.requests import RequestAudit - from pytest_skill_engineering.core.result import ( - DiscoveredSkill, - EvalResult, - SkillDiscovery, - ToolCall, - Turn, - UsageInfo, + from pytest_skill_engineering.core.result import EvalResult + + if not is_dataclass(cls): + raise TypeError(f"{cls} is not a native report dataclass") + context = f"{cls.__name__} at {path}" + if not isinstance(data, dict): + raise ValueError(f"{context} must be an object") + public_fields = [field for field in fields(cls) if not field.name.startswith("_")] + unexpected = data.keys() - {field.name for field in public_fields} + if unexpected: + raise ValueError(f"{context} has unexpected fields: {sorted(unexpected)}") + annotations = get_type_hints( + cls, + globalns={ + **vars(sys.modules[cls.__module__]), + "RequestAudit": RequestAudit, + "EvalResult": EvalResult, + }, ) - from pytest_skill_engineering.reporting.collector import SuiteReport, TestReport - - # Reconstruct tests - raw_tests = _require_key(data, "tests", context="SuiteReport") - if not isinstance(raw_tests, list): - raise ValueError("SuiteReport field 'tests' must be a list") - - tests = [] - for test_data in raw_tests: - if not isinstance(test_data, dict): - raise ValueError("SuiteReport tests must contain objects") - - eval_result = None - raw_eval_result = _require_key( - test_data, - "eval_result", - context=f"TestReport {test_data!r}", + values: dict[str, Any] = {} + for field in public_fields: + if field.name not in data: + raise ValueError(f"{context} is missing required field {field.name!r}") + values[field.name] = _decode_value( + annotations[field.name], data[field.name], path=f"{path}.{field.name}" ) - if raw_eval_result is not None: - if not isinstance(raw_eval_result, dict): - raise ValueError("TestReport field 'eval_result' must be an object or null") - ar_data = raw_eval_result - - # Reconstruct turns - turns = [] - for turn_data in ar_data.get("turns", []): - # Reconstruct tool calls - tool_calls = [] - for tc_data in turn_data.get("tool_calls", []): - # Decode base64 image content if present - image_content = None - if tc_data.get("image_content") is not None: - image_content = base64.b64decode(tc_data["image_content"], validate=True) - - tool_calls.append( - ToolCall( - name=_require_key(tc_data, "name", context="ToolCall"), - arguments=tc_data.get("arguments", {}), - result=tc_data.get("result"), - error=tc_data.get("error"), - duration_ms=tc_data.get("duration_ms"), - image_content=image_content, - image_media_type=tc_data.get("image_media_type"), - call_id=tc_data.get("call_id"), - completion_received=tc_data.get("completion_received"), - success=tc_data.get("success"), - ) - ) - - turns.append( - Turn( - role=_require_key(turn_data, "role", context="Turn"), - content=_require_key(turn_data, "content", context="Turn"), - tool_calls=tool_calls, - ) - ) - - # Reconstruct clarification stats if present - from pytest_skill_engineering.core.result import ClarificationStats - - clarification_stats = None - if ar_data.get("clarification_stats") is not None: - cs_data = ar_data["clarification_stats"] - clarification_stats = ClarificationStats( - count=cs_data.get("count", 0), - turn_indices=cs_data.get("turn_indices", []), - examples=cs_data.get("examples", []), - ) - - # Reconstruct assertions if present - from pytest_skill_engineering.core.result import Assertion - - assertions = [] - for a_data in ar_data.get("assertions", []): - assertions.append( - Assertion( - type=a_data["type"], - passed=a_data["passed"], - message=a_data["message"], - details=a_data.get("details"), - ) - ) - - # Reconstruct available tools if present - from pytest_skill_engineering.core.result import ( - MCPPrompt, - MCPPromptArgument, - SkillInfo, - ToolInfo, - ) - - available_tools = [] - for t_data in ar_data.get("available_tools", []): - available_tools.append( - ToolInfo( - name=_require_key(t_data, "name", context="ToolInfo"), - description=_require_key(t_data, "description", context="ToolInfo"), - input_schema=_require_key(t_data, "input_schema", context="ToolInfo"), - server_name=_require_key(t_data, "server_name", context="ToolInfo"), - ) - ) - - # Reconstruct MCP prompts if present - mcp_prompts = [] - for p_data in ar_data.get("mcp_prompts", []): - args = [ - MCPPromptArgument( - name=a["name"], - description=a.get("description", ""), - required=a.get("required", False), - ) - for a in p_data.get("arguments", []) - ] - mcp_prompts.append( - MCPPrompt( - name=_require_key(p_data, "name", context="MCPPrompt"), - description=p_data.get("description", ""), - arguments=args, - ) - ) - - # Reconstruct skill info if present - skill_info = None - si_data = ar_data.get("skill_info") - if si_data: - skill_info = SkillInfo( - name=_require_key(si_data, "name", context="SkillInfo"), - description=_require_key(si_data, "description", context="SkillInfo"), - instruction_content=si_data.get("instruction_content", ""), - reference_names=si_data.get("reference_names", []), - ) - - # Reconstruct custom agent info if present - from pytest_skill_engineering.core.result import CustomAgentInfo, InstructionFileInfo - - custom_agent_info = None - ca_data = ar_data.get("custom_agent_info") - if ca_data: - custom_agent_info = CustomAgentInfo( - name=_require_key(ca_data, "name", context="CustomAgentInfo"), - description=ca_data.get("description", ""), - file_path=ca_data.get("file_path", ""), - ) - - # Reconstruct instruction files if present - instruction_files = [] - for if_data in ar_data.get("instruction_files", []): - instruction_files.append( - InstructionFileInfo( - name=_require_key(if_data, "name", context="InstructionFileInfo"), - file_path=if_data.get("file_path", ""), - apply_to=if_data.get("apply_to", ""), - description=if_data.get("description", ""), - ) - ) - - # Reconstruct agent result - discovery_data = _require_key(ar_data, "skill_discovery", context="EvalResult") - skill_discovery = None - if discovery_data is not None: - skill_discovery = SkillDiscovery( - skills=[ - DiscoveredSkill(**skill) - for skill in _require_key( - discovery_data, "skills", context="SkillDiscovery" - ) - ], - warnings=_require_key(discovery_data, "warnings", context="SkillDiscovery"), - errors=_require_key(discovery_data, "errors", context="SkillDiscovery"), - complete=_require_key(discovery_data, "complete", context="SkillDiscovery"), - ) - eval_result = EvalResult( - turns=turns, - success=_require_key(ar_data, "success", context="EvalResult"), - error=ar_data.get("error"), - duration_ms=_require_key(ar_data, "duration_ms", context="EvalResult"), - token_usage=_require_key(ar_data, "token_usage", context="EvalResult"), - cost_usd=_require_key(ar_data, "cost_usd", context="EvalResult"), - session_context_count=_require_key( - ar_data, "session_context_count", context="EvalResult" - ), - clarification_stats=clarification_stats, - assertions=assertions, - available_tools=available_tools, - skill_info=skill_info, - skill_discovery=skill_discovery, - effective_system_prompt=_require_key( - ar_data, "effective_system_prompt", context="EvalResult" - ), - mcp_prompts=mcp_prompts, - prompt_name=ar_data.get("prompt_name"), - custom_agent_info=custom_agent_info, - premium_requests=_require_key(ar_data, "premium_requests", context="EvalResult"), - instruction_files=instruction_files, - configuration=ar_data.get("configuration", {}), - capture_errors=ar_data.get("capture_errors", []), - evidence_complete=ar_data.get("evidence_complete"), - request_audit=[ - RequestAudit(**record) for record in ar_data.get("request_audit", []) - ], - stop_reason=ar_data.get("stop_reason"), - usage=[UsageInfo(**record) for record in ar_data.get("usage", [])], - tool_calls_admitted=ar_data.get("tool_calls_admitted", 0), + return cls(**values) + + +def _decode_value(annotation: Any, value: Any, *, path: str) -> Any: + origin = get_origin(annotation) + arguments = get_args(annotation) + if annotation is Any: + return value + if origin in (Union, UnionType): + if value is None and type(None) in arguments: + return None + alternatives = [item for item in arguments if item is not type(None)] + if len(alternatives) != 1: + raise TypeError(f"Unsupported native report field type at {path}: {annotation}") + return _decode_value(alternatives[0], value, path=path) + if origin is Literal: + if value not in arguments: + raise ValueError(f"{path} must be one of {arguments!r}") + return value + if origin in (list, tuple): + if not isinstance(value, list): + raise ValueError(f"{path} must be a list") + if origin is list: + return [ + _decode_value(arguments[0], item, path=f"{path}[{index}]") + for index, item in enumerate(value) + ] + if len(value) != len(arguments): + raise ValueError(f"{path} must contain {len(arguments)} values") + return tuple( + _decode_value(kind, item, path=f"{path}[{index}]") + for index, (kind, item) in enumerate(zip(arguments, value, strict=True)) + ) + if origin is dict: + if not isinstance(value, dict): + raise ValueError(f"{path} must be an object") + return { + _decode_value(arguments[0], key, path=f"{path} key"): _decode_value( + arguments[1], item, path=f"{path}[{key!r}]" ) + for key, item in value.items() + } + if isinstance(annotation, type) and is_dataclass(annotation): + return _decode_dataclass(annotation, value, path=path) + if annotation is bytes: + if not isinstance(value, str): + raise ValueError(f"{path} must be a base64 string") + try: + return base64.b64decode(value, validate=True) + except (ValueError, binascii.Error) as exc: + raise ValueError(f"{path} contains invalid base64 image data") from exc + if annotation is float: + if type(value) not in (int, float): + raise ValueError(f"{path} must be a number") + return float(value) + if annotation in (str, int, bool) and type(value) is annotation: + return value + raise ValueError(f"{path} must be {annotation.__name__}") - agent_id = _require_key(test_data, "agent_id", context="TestReport") - eval_name = _require_key(test_data, "eval_name", context="TestReport") - model = _require_key(test_data, "model", context="TestReport") - system_prompt_name = test_data.get("system_prompt_name") - skill_name = test_data.get("skill_name") - # Reconstruct test report - test_report = TestReport( - name=_require_key(test_data, "name", context="TestReport"), - outcome=_require_key(test_data, "outcome", context="TestReport"), - duration_ms=_require_key(test_data, "duration_ms", context="TestReport"), - eval_result=eval_result, - error=test_data.get("error"), - assertions=test_data.get("assertions", []), - docstring=test_data.get("docstring"), - class_docstring=test_data.get("class_docstring"), - agent_id=agent_id, - eval_name=eval_name, - model=model, - system_prompt_name=system_prompt_name, - skill_name=skill_name, - iteration=test_data.get("iteration"), - properties=[(key, value) for key, value in test_data.get("properties", [])], - ) - tests.append(test_report) +def deserialize_suite_report(data: dict[str, Any]) -> SuiteReport: + """Load every current producer field; missing evidence is never inferred.""" + from pytest_skill_engineering.reporting.collector import SuiteReport - # Reconstruct suite report - return SuiteReport( - name=_require_key(data, "name", context="SuiteReport"), - timestamp=_require_key(data, "timestamp", context="SuiteReport"), - duration_ms=_require_key(data, "duration_ms", context="SuiteReport"), - tests=tests, - passed=_require_key(data, "passed", context="SuiteReport"), - failed=_require_key(data, "failed", context="SuiteReport"), - skipped=_require_key(data, "skipped", context="SuiteReport"), - suite_docstring=data.get("suite_docstring"), - models_without_pricing=_require_key(data, "models_without_pricing", context="SuiteReport"), - ) + payload = {key: value for key, value in data.items() if key != "schema_version"} + return _decode_dataclass(SuiteReport, payload, path="suite") diff --git a/src/pytest_skill_engineering/execution/cost.py b/src/pytest_skill_engineering/execution/cost.py index 2c68e11..b6524ee 100644 --- a/src/pytest_skill_engineering/execution/cost.py +++ b/src/pytest_skill_engineering/execution/cost.py @@ -124,6 +124,11 @@ def _find_pricing_toml(current_dir: Path) -> Path | None: # ── Public API ─────────────────────────────────────────────────────────────── +def unpriced_models(models: set[str]) -> list[str]: + """Find missing prices for this execution's usage, independent of earlier suites.""" + return sorted(models - _load_user_overrides().keys()) + + def estimate_cost( model: str, input_tokens: int, diff --git a/src/pytest_skill_engineering/execution/servers.py b/src/pytest_skill_engineering/execution/servers.py index 5097719..03951ea 100644 --- a/src/pytest_skill_engineering/execution/servers.py +++ b/src/pytest_skill_engineering/execution/servers.py @@ -540,15 +540,15 @@ async def _run_command(self, args: str) -> dict[str, Any]: # Build shell command if self._shell == "none": # Remove grouping quotes without treating Windows path separators as escapes. - lexer = shlex.shlex(self.config.command, posix=True) - lexer.whitespace_split = True - lexer.commenters = "" - if sys.platform == "win32": - lexer.escape = "" - base_parts = list(lexer) - if args: - base_parts.extend(shlex.split(args, posix=True)) - cmd = base_parts + def split_arguments(value: str) -> list[str]: + lexer = shlex.shlex(value, posix=True) + lexer.whitespace_split = True + lexer.commenters = "" + if sys.platform == "win32": + lexer.escape = "" + return list(lexer) + + cmd = split_arguments(self.config.command) + split_arguments(args) elif self._shell in ("powershell", "pwsh"): shell_exe = "powershell" if self._shell == "powershell" else "pwsh" cmd = [shell_exe, "-NoProfile", "-NonInteractive", "-Command", full_cmd] diff --git a/src/pytest_skill_engineering/plugin.py b/src/pytest_skill_engineering/plugin.py index 371729d..eced523 100644 --- a/src/pytest_skill_engineering/plugin.py +++ b/src/pytest_skill_engineering/plugin.py @@ -36,6 +36,7 @@ COLLECTOR_KEY = pytest.StashKey[list[TestReport]]() # Key for storing session messages for @pytest.mark.session SESSION_MESSAGES_KEY = pytest.StashKey[dict[str, list[dict[str, Any]]]]() +PHASE_REPORTS_KEY = pytest.StashKey[dict[str, "PytestTestReport"]]() # Export for use in fixtures and downstream consumers __all__ = [ "COLLECTOR_KEY", @@ -119,6 +120,10 @@ def pytest_configure(config: Config) -> None: "markers", "copilot: mark test as requiring GitHub Copilot SDK credentials", ) + config.addinivalue_line( + "markers", + "aitest_iteration_axis(position): internal repetition parameter-group index", + ) # Always initialize report collection - JSON is always generated config.stash[COLLECTOR_KEY] = [] @@ -139,9 +144,14 @@ def pytest_generate_tests(metafunc: pytest.Metafunc) -> None: if count <= 1: return metafunc.fixturenames.append("_aitest_iteration") + existing = getattr(metafunc, "_calls", []) + position = len(existing[0]._idlist) if existing else 0 metafunc.parametrize( "_aitest_iteration", - range(1, count + 1), + [ + pytest.param(i, marks=pytest.mark.aitest_iteration_axis(position)) + for i in range(1, count + 1) + ], ids=[f"iter-{i}" for i in range(1, count + 1)], indirect=True, ) @@ -172,24 +182,26 @@ def _require_agent_string(agent: Any, *, field_name: str) -> str: raise ValueError(msg) -def _require_provider_model(agent: Any) -> str: +def _require_provider_model(agent: Any) -> str | None: """Read the required provider model from the reporting wrapper.""" provider = getattr(agent, "provider", None) - model = getattr(provider, "model", None) - if isinstance(model, str) and model: + if provider is None or not hasattr(provider, "model"): + raise ValueError("aitest reporting agent is missing required field 'provider.model'") + model = provider.model + if model is None or isinstance(model, str) and model: return model raise ValueError("aitest reporting agent is missing required field 'provider.model'") -def _display_model(model: str) -> str: +def _display_model(model: str | None) -> str | None: """Strip any provider prefix from a model name for display.""" - return model.split("/")[-1] if "/" in model else model + return model.split("/")[-1] if model is not None and "/" in model else model def _build_agent_identity( agent: Any, eval_result: EvalResult, -) -> tuple[str, str, str, str | None, str | None]: +) -> tuple[str, str, str | None, str | None, str | None]: """Resolve stable and human-facing identity fields for report production.""" base_agent_id = _require_agent_string(agent, field_name="id") eval_name = _require_agent_string(agent, field_name="name") @@ -219,23 +231,21 @@ def _build_case_name(item: Item) -> str: if callspec is None or "_aitest_iteration" not in callspec.params: return item.nodeid - param_names = list(callspec.params) param_ids = list(getattr(callspec, "_idlist", [])) - if len(param_names) != len(param_ids): + axis = item.get_closest_marker("aitest_iteration_axis") + if axis is None or len(axis.args) != 1 or type(axis.args[0]) is not int: raise ValueError( "Unable to derive case identity for " - f"{item.nodeid!r}: pytest parameter IDs are unavailable" + f"{item.nodeid!r}: repetition parameter-group identity is unavailable" ) - - base_nodeid, _, _ = item.nodeid.partition("[") - preserved_ids = [ - param_id - for param_name, param_id in zip(param_names, param_ids, strict=True) - if param_name != "_aitest_iteration" - ] - if not preserved_ids: + position = axis.args[0] + if not 0 <= position < len(param_ids): + raise ValueError(f"Invalid repetition parameter-group index for {item.nodeid!r}") + del param_ids[position] + base_nodeid = item.nodeid.removesuffix(f"[{callspec.id}]") + if not param_ids: return base_nodeid - return f"{base_nodeid}[{'-'.join(preserved_ids)}]" + return f"{base_nodeid}[{'-'.join(param_ids)}]" @pytest.hookimpl(hookwrapper=True, tryfirst=True) @@ -247,7 +257,7 @@ def pytest_runtest_makereport(item: Item, call: Any) -> Any: module-scoped agent fixtures that cannot use the function-scoped fixture. """ # Auto-stash CopilotResult before processing (tryfirst ensures this runs early) - if call.when == "call" and not hasattr(item, "_aitest_result"): + if call.when in ("setup", "call") and not hasattr(item, "_aitest_result"): from pytest_skill_engineering.copilot.eval import CopilotEval from pytest_skill_engineering.copilot.fixtures import stash_on_item from pytest_skill_engineering.copilot.result import CopilotResult @@ -261,9 +271,8 @@ def pytest_runtest_makereport(item: Item, call: Any) -> Any: outcome = yield report: PytestTestReport = outcome.get_result() - # Only process call phase (not setup/teardown) - if report.when != "call": - return + phases = item.stash.setdefault(PHASE_REPORTS_KEY, {}) + phases[report.when] = report # Check if reporting is enabled tests = item.config.stash.get(COLLECTOR_KEY, None) @@ -280,6 +289,14 @@ def pytest_runtest_makereport(item: Item, call: Any) -> Any: # Tests without eval execution do not create eval reports. if not runs: return + if report.when != "teardown": + if report.when == "call": + for eval_result, agent in runs: + _add_junit_properties(report, eval_result, agent) + return + failed_phases = [phase for phase in phases.values() if phase.failed] + skipped_phases = [phase for phase in phases.values() if phase.skipped] + final_outcome = "failed" if failed_phases else "skipped" if skipped_phases else "passed" # Get test function docstring if available docstring = None @@ -302,9 +319,10 @@ def pytest_runtest_makereport(item: Item, call: Any) -> Any: # Capture error message — just the assertion/exception, never raw tracebacks. # Tracebacks contain file paths, line numbers, and nodeids that pollute # User-facing reports. - error_msg = None - if report.failed: - error_text = str(report.longrepr) + phase_errors = [] + for failed_report in failed_phases: + error_msg = None + error_text = str(failed_report.longrepr) error_lines = error_text.split("\n") # Extract lines starting with "E " — pytest's assertion/exception lines @@ -319,6 +337,11 @@ def pytest_runtest_makereport(item: Item, call: Any) -> Any: if stripped: error_msg = stripped break + if error_msg is not None: + phase_errors.append( + error_msg if failed_report.when == "call" else f"{failed_report.when}: {error_msg}" + ) + error_msg = "\n".join(phase_errors) or None # Detect iteration index from _aitest_iteration fixture iteration: int | None = None @@ -333,8 +356,12 @@ def pytest_runtest_makereport(item: Item, call: Any) -> Any: ) test_report = TestReport( name=_build_case_name(item), - outcome=report.outcome, - duration_ms=eval_result.duration_ms if len(runs) > 1 else report.duration * 1000, + outcome=final_outcome, + duration_ms=( + eval_result.duration_ms + if len(runs) > 1 + else sum(phase.duration for phase in phases.values()) * 1000 + ), eval_result=eval_result, error=error_msg, properties=properties, @@ -382,7 +409,8 @@ def _add_junit_properties( agent, eval_result ) props.append(("aitest.agent.name", eval_name)) - props.append(("aitest.model", model)) + if model is not None: + props.append(("aitest.model", model)) if system_prompt_name: props.append(("aitest.prompt", system_prompt_name)) diff --git a/src/pytest_skill_engineering/reporting/collector.py b/src/pytest_skill_engineering/reporting/collector.py index f1ccd5c..1ee940e 100644 --- a/src/pytest_skill_engineering/reporting/collector.py +++ b/src/pytest_skill_engineering/reporting/collector.py @@ -42,7 +42,7 @@ class TestReport: # Eval identity (populated by plugin from Eval object) agent_id: str = "" eval_name: str = "" - model: str = "" + model: str | None = None system_prompt_name: str | None = None skill_name: str | None = None iteration: int | None = None @@ -181,9 +181,15 @@ def build_suite_report( skipped = sum(1 for t in tests if t.outcome == "skipped") total_duration = sum(t.duration_ms for t in tests) - from pytest_skill_engineering.execution.cost import models_without_pricing + from pytest_skill_engineering.execution.cost import unpriced_models - suite_models = {test.model for test in tests} + suite_models = { + usage.model + for test in tests + if test.eval_result is not None + for result in test.eval_result.execution_results + for usage in result.usage + } return SuiteReport( name=name, timestamp=datetime.now().isoformat(), @@ -193,5 +199,5 @@ def build_suite_report( failed=failed, skipped=skipped, suite_docstring=suite_docstring, - models_without_pricing=sorted(suite_models & models_without_pricing), + models_without_pricing=unpriced_models(suite_models), ) diff --git a/tests/contracts/test_native_evidence.py b/tests/contracts/test_native_evidence.py index 47e07a3..26fc049 100644 --- a/tests/contracts/test_native_evidence.py +++ b/tests/contracts/test_native_evidence.py @@ -103,6 +103,7 @@ def test_model_and_configuration_variants_remain_separate_without_ranking(tmp_pa other = _suite().tests[0] other.model = "model-two" assert other.eval_result is not None + assert other.eval_result.configuration is not None other.eval_result.configuration["model"] = "model-two" other.eval_result.effective_system_prompt = "Different system prompt." suite = build_suite_report([*suite.tests, other], name="Configuration variants") diff --git a/tests/contracts/test_review_regressions.py b/tests/contracts/test_review_regressions.py new file mode 100644 index 0000000..b7f284d --- /dev/null +++ b/tests/contracts/test_review_regressions.py @@ -0,0 +1,365 @@ +"""Regression contracts for the whole-repository review; no model calls.""" + +from __future__ import annotations + +import base64 +import json +import shlex +import sys +from inspect import isawaitable +from pathlib import Path +from typing import Any +from uuid import uuid4 + +import pytest +from copilot.generated.session_events import SessionEvent +from copilot.tools import ToolInvocation + +from pytest_skill_engineering.copilot.config import snapshot_session_configuration +from pytest_skill_engineering.copilot.contracts import CopilotEvalConfig, SubagentInvocation +from pytest_skill_engineering.copilot.eval import CopilotEval +from pytest_skill_engineering.copilot.events import EventMapper +from pytest_skill_engineering.copilot.fixtures import _convert_to_aitest +from pytest_skill_engineering.copilot.personas import ( + _inject_skill_reference_tools, + _make_subagent_dispatch_tool, +) +from pytest_skill_engineering.copilot.result import CopilotResult +from pytest_skill_engineering.core.evals import load_custom_agent, load_custom_agents +from pytest_skill_engineering.core.result import ToolCall, Turn, UsageInfo +from pytest_skill_engineering.core.serialization import serialize_dataclass +from pytest_skill_engineering.execution.servers import CLIServer, CLIServerProcess +from pytest_skill_engineering.reporting import build_suite_report, generate_json, load_suite_report +from pytest_skill_engineering.reporting.collector import TestReport as CaseReport + + +def test_agent_settings_are_shared_across_loaders(tmp_path: Path) -> None: + definition = ( + "---\nname: declared-name\ndescription: Specialist\nmodel: child-model\n" + "reasoning_effort: high\ntools: []\ninfer: false\nskills: [specialist]\n" + "mcp-servers:\n helper:\n command: helper\n args: []\n---\nDo the task." + ) + plugin = tmp_path / "plugin" + (plugin / "agents").mkdir(parents=True) + (plugin / "plugin.json").write_text('{"name":"review-plugin"}', encoding="utf-8") + agent_file = plugin / "agents" / "filename.agent.md" + agent_file.write_text(definition, encoding="utf-8") + expected = load_custom_agent(agent_file) + assert expected["name"] == "declared-name" + assert expected["tools"] == [] + assert expected["mcp_servers"]["helper"]["command"] == "helper" + assert CopilotEval.from_plugin(plugin).custom_agents == [expected] + for folder, factory in ( + (".github", CopilotEval.from_copilot_config), + (".claude", CopilotEval.from_claude_config), + ): + directory = tmp_path / folder / "agents" + directory.mkdir(parents=True) + (directory / "filename.agent.md").write_text(definition, encoding="utf-8") + assert factory(tmp_path).custom_agents == [expected] + assert load_custom_agents(agent_file.parent, include={"declared-name"}) == [expected] + (agent_file.parent / "another.agent.md").write_text(definition, encoding="utf-8") + with pytest.raises(ValueError, match="Duplicate custom agent name"): + load_custom_agents(agent_file.parent) + with pytest.raises(ValueError, match="Duplicate custom agent name"): + CopilotEval.from_plugin(plugin) + + +def _skill(parent: Path, name: str, reference: str = "commands/guide.md") -> Path: + directory = parent / name + directory.mkdir(parents=True) + (directory / "SKILL.md").write_text( + f"---\nname: {name}\ndescription: Fixture\n---\nInstructions.", + encoding="utf-8", + ) + path = directory / "references" / reference + path.parent.mkdir(parents=True) + path.write_text(f"Instructions for {name}", encoding="utf-8") + return directory + + +@pytest.mark.parametrize("parent_directory", [False, True]) +async def test_disabled_and_nested_references(tmp_path: Path, parent_directory: bool) -> None: + enabled = _skill(tmp_path, "enabled") + disabled = _skill(tmp_path, "disabled", "secret.md") + agent = CopilotEval( + skill_directories=[str(tmp_path)] if parent_directory else [str(enabled), str(disabled)], + extra_config={"disabled_skills": ["disabled"]}, + ) + config = agent.build_session_config() + _inject_skill_reference_tools(agent, config) + tools = {tool.name: tool for tool in config["tools"]} + enum = tools["read_skill_reference"].parameters["properties"]["filename"]["enum"] + assert enum == ["commands/guide.md"] + assert "secret.md" not in config["system_message"]["content"] + result = await tools["read_skill_reference"].handler( + ToolInvocation(tool_name="read_skill_reference", arguments={"filename": enum[0]}) + ) + assert result.text_result_for_llm == "Instructions for enabled" + rejected = await tools["read_skill_reference"].handler( + ToolInvocation(tool_name="read_skill_reference", arguments={"filename": "secret.md"}) + ) + assert rejected.result_type == "failure" + config["disabled_skills"] = ["enabled", "disabled"] + config.pop("tools") + config.pop("system_message") + _inject_skill_reference_tools(agent, config) + assert "tools" not in config + assert "system_message" not in config + + +@pytest.mark.skipif(sys.platform != "win32", reason="Windows direct argument parsing") +@pytest.mark.parametrize("arguments", [r"C:\work\input.txt", r'"C:\work folder\input.txt" ""']) +async def test_direct_cli_windows_paths(arguments: str) -> None: + server = CLIServerProcess( + CLIServer( + tool_prefix="echo", + command=f"{shlex.quote(sys.executable)} -c 'import sys,json; print(json.dumps(sys.argv[1:]))'", + shell="none", + ) + ) + result: dict[str, Any] = json.loads(await server.call_tool("echo_execute", {"args": arguments})) + assert result["exit_code"] == 0, result + assert json.loads(result["stdout"]) == ( + [r"C:\work\input.txt"] if arguments.startswith("C:") else [r"C:\work folder\input.txt", ""] + ) + + +def _event(kind: str, **data: Any) -> SessionEvent: + return SessionEvent.from_dict( + {"id": str(uuid4()), "type": kind, "timestamp": "2026-10-01T08:00:00Z", "data": data} + ) + + +@pytest.mark.parametrize("field", ["contents", "binaryResultsForLlm"]) +def test_sdk_images_are_captured_and_round_trip(tmp_path: Path, field: str) -> None: + mapper = EventMapper() + mapper.handle( + _event("tool.execution_start", toolCallId="image-1", toolName="image", arguments={}) + ) + payloads = [b"first-image", b"", b"third-image"] + completion = _event( + "tool.execution_complete", + toolCallId="image-1", + success=True, + result={ + "content": "", + field: [ + {"type": "image", "data": base64.b64encode(data).decode(), "mimeType": "image/png"} + for data in payloads + ], + }, + ) + mapper.handle(completion) + mapper.handle(completion) + result = mapper.build() + call = result.all_tool_calls[0] + assert result.evidence_complete + assert call.image_content == payloads[0] + assert [image.data for image in call.additional_images] == payloads[1:] + converted = _convert_to_aitest(CopilotEval(), result) + assert converted is not None + suite = build_suite_report([CaseReport("image-test", "passed", 1, converted[0])], "Images") + output = tmp_path / "images.json" + generate_json(suite, output) + restored = load_suite_report(output) + assert serialize_dataclass(restored) == serialize_dataclass(suite) + captured = restored.tests[0].eval_result + assert captured is not None + assert [image.data for image in captured.tool_images_for("image")] == payloads + + +def test_invalid_image_is_an_explicit_capture_error() -> None: + mapper = EventMapper() + mapper.handle( + _event("tool.execution_start", toolCallId="image-1", toolName="image", arguments={}) + ) + mapper.handle( + _event( + "tool.execution_complete", + toolCallId="image-1", + success=True, + result={ + "content": "", + "contents": [{"type": "image", "data": "!", "mimeType": "image/png"}], + }, + ) + ) + result = mapper.build() + assert not result.evidence_complete + assert not result.success + assert any("invalid image" in error for error in result.capture_errors) + + +@pytest.mark.parametrize("successful", [True, False]) +async def test_child_evidence_is_separate_and_counted_once( + tmp_path: Path, successful: bool +) -> None: + child_agent = CopilotEval(name="child", model="child-model") + grandchild = CopilotResult( + agent=CopilotEval(name="grandchild", model="grandchild-model"), + usage=[UsageInfo("grandchild-model", 7, 3)], + total_premium_requests=1, + ) + child = CopilotResult( + agent=child_agent, + success=successful, + error=None if successful else "child failed", + capture_errors=[] if successful else ["incomplete child"], + usage=[UsageInfo("child-model", 100, 20)], + turns=[ + Turn( + "assistant", + "child output", + [ + ToolCall( + "write", + {}, + result="child receipt", + call_id="write-1", + success=True, + completion_received=True, + ), + ], + ) + ], + configuration={"model": "child-model", "instructions": "child instructions"}, + subagent_invocations=[ + SubagentInvocation("grandchild-1", "grandchild", "completed", result=grandchild) + ], + total_premium_requests=2, + ) + + async def nested(agent: CopilotEvalConfig, prompt: str) -> CopilotResult: + return child + + parent = CopilotEval(name="parent", model="parent-model") + mapper = EventMapper() + mapper.handle(_event("assistant.usage", model="parent-model", inputTokens=10, outputTokens=5)) + tool = _make_subagent_dispatch_tool( + "runSubagent", parent, [{"name": "child", "prompt": "Child instructions"}], mapper, nested + ) + assert tool.handler is not None + pending = tool.handler( + ToolInvocation( + tool_call_id="dispatch-1", + tool_name="runSubagent", + arguments={"agentSlug": "child", "prompt": "Do the task"}, + ) + ) + assert isawaitable(pending) + outcome = await pending + result = mapper.build() + assert outcome.result_type == ("success" if successful else "failure") + assert result.usage[0].input_tokens == 10 + assert result.total_tokens == 145 + assert result.total_premium_requests == 2 + assert result.all_tool_calls == [] + assert result.evidence_complete is successful + assert result.subagent_invocations[0].result is child + converted = _convert_to_aitest(parent, result) + assert converted is not None + suite = build_suite_report([CaseReport("child-test", "passed", 1, converted[0])], "Children") + output = tmp_path / "children.json" + generate_json(suite, output) + restored = load_suite_report(output) + assert serialize_dataclass(restored) == serialize_dataclass(suite) + saved = restored.tests[0].eval_result + assert saved is not None + assert len(saved.execution_results) == 3 + assert saved.token_usage["total"] == 145 + saved_child = saved.subagent_invocations[0].result + assert saved_child is not None + assert saved_child.all_tool_calls[0].result == "child receipt" + child.usage[0].input_tokens = None + assert result.total_input_tokens is None + assert result.total_tokens is None + + +def test_prepared_settings_are_saved_not_reconstructed() -> None: + agent = CopilotEval( + model="configured-model", + instructions="Original prompt", + allowed_tools=["read"], + extra_config={ + "model": "runtime-model", + "available_tools": ["write"], + "system_message": {"mode": "replace", "content": "Runtime prompt"}, + }, + ) + configuration = snapshot_session_configuration(agent, agent.build_session_config()) + result = CopilotResult(model_used="runtime-model", configuration=configuration) + converted = _convert_to_aitest(agent, result) + assert converted is not None + assert converted[0].configuration["model"] == "runtime-model" + assert converted[0].configuration["allowed_tools"] == ["write"] + assert converted[0].effective_system_prompt == "Runtime prompt" + configuration["allowed_tools"].append("extra") + assert converted[0].configuration["allowed_tools"] == ["write"] + + +def test_unknown_model_is_null_and_unavailable_settings_remain_missing(tmp_path: Path) -> None: + converted = _convert_to_aitest( + CopilotEval(), CopilotResult(success=False, error="startup failed") + ) + assert converted is not None + assert converted[1].provider.model is None + assert converted[0].configuration is None + assert converted[0].effective_system_prompt is None + suite = build_suite_report( + [CaseReport("startup", "failed", 1, converted[0], model=None)], "Unknown" + ) + output = tmp_path / "unknown.json" + generate_json(suite, output) + assert load_suite_report(output).tests[0].model is None + + +@pytest.mark.parametrize( + "field", + [ + "configuration", + "capture_errors", + "evidence_complete", + "request_audit", + "usage", + "tool_calls_admitted", + "subagent_invocations", + "stop_reason", + "turns", + ], +) +def test_current_schema_rejects_missing_evidence(tmp_path: Path, field: str) -> None: + converted = _convert_to_aitest(CopilotEval(), CopilotResult()) + assert converted is not None + suite = build_suite_report([CaseReport("missing", "passed", 1, converted[0])], "Missing") + path = tmp_path / "missing.json" + generate_json(suite, path) + data = json.loads(path.read_text(encoding="utf-8")) + del data["tests"][0]["eval_result"][field] + path.write_text(json.dumps(data), encoding="utf-8") + with pytest.raises(ValueError, match=f"missing required field '{field}'"): + load_suite_report(path) + + +def test_all_usage_models_contribute_pricing_warnings( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.chdir(tmp_path) + pricing = tmp_path / "pricing.toml" + pricing.write_text("[models.last-model]\ninput = 1\noutput = 2\n", encoding="utf-8") + agent = CopilotEval(model="last-model") + result = CopilotResult( + model_used="last-model", + usage=[UsageInfo("first-model", 100, 20), UsageInfo("last-model", 50, 10)], + ) + converted = _convert_to_aitest(agent, result) + assert converted is not None + suite = build_suite_report( + [CaseReport("pricing", "passed", 1, converted[0], model="last-model")], "Pricing" + ) + assert suite.models_without_pricing == ["first-model"] + pricing.write_text( + "[models.first-model]\ninput = 3\noutput = 4\n[models.last-model]\ninput = 1\noutput = 2\n", + encoding="utf-8", + ) + assert build_suite_report(suite.tests, "Updated pricing").models_without_pricing == [] diff --git a/tests/contracts/test_usage_evidence.py b/tests/contracts/test_usage_evidence.py index 5187906..b484d1e 100644 --- a/tests/contracts/test_usage_evidence.py +++ b/tests/contracts/test_usage_evidence.py @@ -230,13 +230,14 @@ def test_multiple_runs_and_verification_properties_reach_native_report( from pytest_skill_engineering.copilot.eval import CopilotEval from pytest_skill_engineering.copilot.fixtures import stash_on_item from pytest_skill_engineering.copilot.result import CopilotResult + from pytest_skill_engineering.copilot.config import snapshot_session_configuration def test_comparison(request, record_property): record_property("verification", {"status": "failed", "artifact": "expected.txt"}) for name in ("baseline", "treatment"): - stash_on_item(request.node, - CopilotEval(name=name, model="test-model", instructions=name), - CopilotResult(success=True)) + agent = CopilotEval(name=name, model="test-model", instructions=name) + configuration = snapshot_session_configuration(agent, agent.build_session_config()) + stash_on_item(request.node, agent, CopilotResult(success=True, configuration=configuration)) assert False, "Independent file check failed" """ ) @@ -308,10 +309,15 @@ def test_configuration_is_a_snapshot_without_connection_secrets() -> None: } }, ) - converted = _convert_to_aitest(agent, EventMapper().build()) + from pytest_skill_engineering.copilot.config import snapshot_session_configuration + + result = EventMapper().build() + result.configuration = snapshot_session_configuration(agent, agent.build_session_config()) + converted = _convert_to_aitest(agent, result) assert converted is not None tools.append("write") configuration = converted[0].configuration + assert configuration is not None assert configuration["allowed_tools"] == ["read"] assert configuration["mcp_servers"]["documents"]["tools"] == ["read"] assert "synthetic-secret" not in json.dumps(configuration) @@ -381,8 +387,8 @@ def test_comparison(request): for test, role in zip(saved["tests"], ("baseline", "treatment"), strict=True): assert test["eval_name"] == f"helper ({role})" assert test["agent_id"] == f"helper ({role})" - assert test["eval_result"]["configuration"]["name"] == "helper" - assert test["eval_result"]["configuration"]["comparison_role"] == role + assert test["eval_result"]["configuration"] is None + assert test["eval_result"]["comparison_role"] == role def test_tool_repr_preserves_existing_status_format() -> None: diff --git a/tests/integration/copilot/test_12_custom_agents.py b/tests/integration/copilot/test_12_custom_agents.py index 583ba24..059df72 100644 --- a/tests/integration/copilot/test_12_custom_agents.py +++ b/tests/integration/copilot/test_12_custom_agents.py @@ -76,6 +76,22 @@ async def test_test_writer_agent_creates_test_file(self, copilot_eval, tmp_path) invocation.name == "test-writer" and invocation.status == "completed" for invocation in result.subagent_invocations ), "The test-writer must complete rather than recursively dispatching itself" + children = [ + invocation.result + for invocation in result.subagent_invocations + if invocation.result is not None + ] + assert children, "Framework-owned child evidence must be retained" + assert all(child.usage and child.configuration is not None for child in children) + usages = result.all_usage + assert all( + usage.input_tokens is not None and usage.output_tokens is not None for usage in usages + ), "Live accounting must retain unknown values instead of inventing totals" + assert result.total_tokens == sum( + usage.input_tokens + usage.output_tokens + for usage in usages + if usage.input_tokens is not None and usage.output_tokens is not None + ) async def test_docs_writer_agent_creates_readme(self, copilot_eval, tmp_path): """Custom docs-writer agent produces a README.md for the project.""" diff --git a/tests/integration/copilot/test_13_plugins.py b/tests/integration/copilot/test_13_plugins.py index 5c1a4bf..23fcd31 100644 --- a/tests/integration/copilot/test_13_plugins.py +++ b/tests/integration/copilot/test_13_plugins.py @@ -14,6 +14,7 @@ from pathlib import Path import pytest +from copilot.tools import Tool, ToolInvocation, ToolResult from pytest_skill_engineering.copilot.eval import CopilotEval @@ -120,17 +121,67 @@ class TestCopilotPluginExecution: async def test_plugin_eval_creates_output(self, copilot_eval, tmp_path): """CopilotEval.from_plugin() produces a working eval that can execute tasks.""" + balances = {"checking": "$125.00", "savings": "$250.00"} + receipt = tmp_path / "balances.txt" + + async def get_balance(invocation: ToolInvocation) -> ToolResult: + account = (invocation.arguments or {}).get("account_type") + if not isinstance(account, str) or account not in balances: + return ToolResult( + text_result_for_llm="Error: account_type must be checking or savings.", + result_type="failure", + ) + entry = f"{account}: {balances[account]}" + with receipt.open("a", encoding="utf-8") as output: + output.write(entry + "\n") + return ToolResult(text_result_for_llm=entry) + agent = CopilotEval.from_plugin( PLUGIN_DIR, model=DEFAULT_MODEL, working_directory=str(tmp_path), + excluded_tools=["task"], + extra_config={ + "tools": [ + Tool( + name="get_balance", + description="Look up an account balance and save it to balances.txt.", + parameters={ + "type": "object", + "properties": { + "account_type": { + "type": "string", + "enum": ["checking", "savings"], + } + }, + "required": ["account_type"], + }, + handler=get_balance, + ) + ] + }, ) result = await copilot_eval( agent, - "Create a file called balances.txt that lists checking and savings account types.", + "Have banking-advisor use get_balance for both checking and savings accounts. " + "The banking tool saves each balance to balances.txt.", ) assert result.success, f"Failed: {result.error}" - assert list(tmp_path.rglob("balances.txt")), "balances.txt was not created" + assert set(receipt.read_text(encoding="utf-8").splitlines()) == { + "checking: $125.00", + "savings: $250.00", + } + children = [ + invocation.result + for invocation in result.subagent_invocations + if invocation.name == "banking-advisor" and invocation.result is not None + ] + assert children, "The plugin's banking agent was not dispatched" + assert all( + child.configuration is not None + and child.configuration["allowed_tools"] == ["get_balance", "transfer"] + for child in children + ) async def test_claude_project_eval_runs(self, copilot_eval, tmp_path): """CopilotEval.from_claude_config() produces a working eval.""" diff --git a/tests/integration/copilot/test_events.py b/tests/integration/copilot/test_events.py index a220334..0ee7f3c 100644 --- a/tests/integration/copilot/test_events.py +++ b/tests/integration/copilot/test_events.py @@ -14,13 +14,74 @@ from __future__ import annotations +import base64 +from pathlib import Path + import pytest +from copilot.tools import Tool, ToolBinaryResult, ToolInvocation, ToolResult from pytest_skill_engineering.copilot.eval import CopilotEval +from pytest_skill_engineering.copilot.fixtures import _convert_to_aitest +from pytest_skill_engineering.reporting import build_suite_report, generate_json, load_suite_report +from pytest_skill_engineering.reporting.collector import TestReport as CaseReport from .conftest import DEFAULT_MODEL +@pytest.mark.copilot +async def test_image_receipt_and_runtime_overrides_are_captured(copilot_eval, tmp_path: Path): + png = base64.b64decode( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+aJ6kAAAAASUVORK5CYII=" + ) + + async def image(invocation: ToolInvocation) -> ToolResult: + (tmp_path / "receipt.png").write_bytes(png) + return ToolResult( + text_result_for_llm="Saved receipt.png.", + binary_results_for_llm=[ + ToolBinaryResult(data=base64.b64encode(png).decode(), mime_type="image/png") + ], + ) + + instructions = "Use the evidence_image tool exactly once to save receipt.png, then finish." + agent = CopilotEval( + name="image-evidence", + model="configured-but-unused", + client_mode="empty", + instructions="This prompt is overridden.", + allowed_tools=["configured-but-unused"], + working_directory=str(tmp_path), + extra_config={ + "model": DEFAULT_MODEL, + "available_tools": ["evidence_image"], + "system_message": {"mode": "replace", "content": instructions}, + "tools": [ + Tool(name="evidence_image", description="Save a PNG receipt.", handler=image) + ], + }, + ) + result = await copilot_eval(agent, "Save receipt.png using evidence_image.") + assert result.success, result.error + assert (tmp_path / "receipt.png").read_bytes() == png + calls = result.tool_calls_for("evidence_image") + assert len(calls) == 1 + assert calls[0].image_content == png + assert calls[0].image_media_type == "image/png" + assert result.configuration is not None + assert result.configuration["model"] == DEFAULT_MODEL + assert result.configuration["allowed_tools"] == ["evidence_image"] + assert result.configuration["instructions"] == instructions + converted = _convert_to_aitest(agent, result) + assert converted is not None + suite = build_suite_report([CaseReport("image", "passed", 1, converted[0])], "Live image") + output = tmp_path / "evidence.json" + generate_json(suite, output) + saved = load_suite_report(output).tests[0].eval_result + assert saved is not None + assert saved.all_tool_calls[0].image_content == png + assert saved.effective_system_prompt == instructions + + @pytest.mark.copilot class TestReasoningEffort: """reasoning_effort configuration is accepted and run succeeds.""" diff --git a/tests/unit/test_benchmark_controls.py b/tests/unit/test_benchmark_controls.py index 5501ae0..fa4c367 100644 --- a/tests/unit/test_benchmark_controls.py +++ b/tests/unit/test_benchmark_controls.py @@ -691,6 +691,7 @@ async def fail() -> None: async def test_audit_records_in_native_json_with_bound_handler(tmp_path: Any) -> None: + from pytest_skill_engineering.copilot.config import snapshot_session_configuration from pytest_skill_engineering.copilot.requests import RequestAudit from pytest_skill_engineering.copilot.result import CopilotResult, UsageInfo from pytest_skill_engineering.core.serialization import deserialize_suite_report @@ -722,6 +723,7 @@ async def act(self, invocation: ToolInvocation) -> ToolResult: request_audit=[RequestAudit("r1", "actual", ["act"], "medium", ["high"], 1, "abc")], usage=[UsageInfo(model="actual", input_tokens=3, output_tokens=4, reasoning_tokens=2)], tool_calls_admitted=80, + configuration=snapshot_session_configuration(agent, agent.build_session_config()), ) converted = _convert_to_aitest(agent, result) assert converted is not None diff --git a/tests/unit/test_copilot_agent.py b/tests/unit/test_copilot_agent.py index f7e2bba..cfaa63f 100644 --- a/tests/unit/test_copilot_agent.py +++ b/tests/unit/test_copilot_agent.py @@ -167,7 +167,7 @@ def test_without_frontmatter(self, tmp_path): result = _parse_agent_file(f) assert result.get("name") == "my-agent" assert result.get("prompt") == "You are a helpful agent." - assert "description" not in result + assert result.get("description") == "" assert "tools" not in result def test_name_derived_from_filename(self, tmp_path): diff --git a/tests/unit/test_copilot_personas.py b/tests/unit/test_copilot_personas.py index 3a9e400..0d966ba 100644 --- a/tests/unit/test_copilot_personas.py +++ b/tests/unit/test_copilot_personas.py @@ -31,14 +31,13 @@ def skill_dir(tmp_path: Path) -> Path: """Create a skill directory with SKILL.md and references/.""" skill = tmp_path / "my-skill" skill.mkdir() - (skill / "SKILL.md").write_text("---\nname: my-skill\n---\n# My Skill\nInstructions here.") + (skill / "SKILL.md").write_text( + "---\nname: my-skill\ndescription: Reference fixture\n---\n# My Skill\nInstructions here." + ) refs = skill / "references" refs.mkdir() (refs / "guide.md").write_text("# Guide\nDetailed info.") - (refs / "lookup.txt").write_text("key1=value1\nkey2=value2") - (refs / "data.json").write_text('{"items": [1, 2, 3]}') - # Non-matching file extension should be ignored - (refs / "image.png").write_bytes(b"\x89PNG") + (refs / "lookup.md").write_text("key1=value1\nkey2=value2") return skill @@ -47,7 +46,9 @@ def skill_dir_no_refs(tmp_path: Path) -> Path: """Create a skill directory without references/.""" skill = tmp_path / "no-refs-skill" skill.mkdir() - (skill / "SKILL.md").write_text("---\nname: no-refs\n---\n# No Refs") + (skill / "SKILL.md").write_text( + "---\nname: no-refs-skill\ndescription: No references\n---\n# No Refs" + ) return skill @@ -86,8 +87,7 @@ def test_only_supported_extensions(self, skill_dir: Path) -> None: read_tool = next(t for t in config["tools"] if t.name == "read_skill_reference") enum_values = read_tool.parameters["properties"]["filename"]["enum"] assert "guide.md" in enum_values - assert "lookup.txt" in enum_values - assert "data.json" in enum_values + assert "lookup.md" in enum_values assert "image.png" not in enum_values def test_system_message_added(self, skill_dir: Path) -> None: @@ -106,7 +106,9 @@ def test_parent_directory_scanning(self, tmp_path: Path) -> None: parent.mkdir() skill = parent / "child-skill" skill.mkdir() - (skill / "SKILL.md").write_text("---\nname: child\n---\n# Child") + (skill / "SKILL.md").write_text( + "---\nname: child-skill\ndescription: Child fixture\n---\n# Child" + ) refs = skill / "references" refs.mkdir() (refs / "doc.md").write_text("# Doc") diff --git a/tests/unit/test_plugin_hooks.py b/tests/unit/test_plugin_hooks.py index 9245040..e859658 100644 --- a/tests/unit/test_plugin_hooks.py +++ b/tests/unit/test_plugin_hooks.py @@ -43,6 +43,77 @@ def _load_json(path: Path) -> Any: return json.loads(path.read_text(encoding="utf-8")) +def test_grouped_parameter_ids_survive_repetition(pytester: Pytester) -> None: + pytester.makeconftest(CAPTURE_REPORTS_CONFTEST) + pytester.makepyfile( + """ + import pytest + + @pytest.mark.parametrize("a,b", [(1,2), (3,4)], ids=["iter-1", "bracket[x]"]) + @pytest.mark.parametrize("tag", ["a", "b"], ids=["left", "right"]) + def test_grouped(a, b, tag, manual_aitest): + assert b == a + 1 + """ + ) + result = _run_pytest(pytester, "--aitest-iterations=2", "--aitest-json=report.json", "-q") + result.assert_outcomes(passed=8) + collected = _load_json(pytester.path / "report.json")["tests"] + names = {entry["name"] for entry in collected} + assert len(names) == 4 + assert {entry["iteration"] for entry in collected} == {1, 2} + assert all(sum(entry["name"] == name for entry in collected) == 2 for name in names) + assert any("bracket[x]" in name for name in names) + assert any("iter-1" in name for name in names) + assert all("iter-2" not in name for name in names) + + +@pytest.mark.parametrize("phase", ["setup", "teardown"]) +def test_fixture_failure_updates_native_outcome(pytester: Pytester, phase: str) -> None: + fixture = """ +@pytest.fixture +def broken(manual_aitest, record_property): + %s +""" % ( + 'raise RuntimeError("verification failed")' + if phase == "setup" + else 'yield\n record_property("late_verification", {"passed": False})\n' + ' raise RuntimeError("verification failed")' + ) + pytester.makeconftest(CAPTURE_REPORTS_CONFTEST + fixture) + pytester.makepyfile("def test_case(broken):\n assert True\n") + result = _run_pytest(pytester, "--aitest-json=report.json", "-q") + result.assert_outcomes(errors=1, passed=1 if phase == "teardown" else 0) + report = _load_json(pytester.path / "report.json") + assert report["failed"] == 1 + assert report["passed"] == 0 + assert len(report["tests"]) == 1 + case = report["tests"][0] + assert case["outcome"] == "failed" + assert f"{phase}:" in case["error"] + assert "verification failed" in case["error"] + if phase == "teardown": + assert ["late_verification", {"passed": False}] in case["properties"] + + +def test_unknown_model_is_not_invented_in_native_or_junit_reports(pytester: Pytester) -> None: + pytester.makepyfile( + """ + from pytest_skill_engineering.copilot import CopilotEval, CopilotResult + from pytest_skill_engineering.copilot.fixtures import stash_on_item + + def test_unknown(request): + result = CopilotResult(success=False, error="startup failed") + stash_on_item(request.node, CopilotEval(name="unknown"), result) + assert result.error == "startup failed" + """ + ) + result = _run_pytest(pytester, "--aitest-json=report.json", "--junitxml=junit.xml", "-q") + result.assert_outcomes(passed=1) + report = _load_json(pytester.path / "report.json") + assert report["tests"][0]["model"] is None + assert "aitest.model" not in (pytester.path / "junit.xml").read_text(encoding="utf-8") + + def _make_eval_result( *, success: bool = True, diff --git a/tests/unit/test_release_invariants.py b/tests/unit/test_release_invariants.py index e51d346..e16b823 100644 --- a/tests/unit/test_release_invariants.py +++ b/tests/unit/test_release_invariants.py @@ -8,9 +8,9 @@ ROOT = Path(__file__).parents[2] -def test_project_version_is_pinned_to_1_0_2() -> None: +def test_project_version_is_pinned_to_1_0_3() -> None: pyproject = tomllib.loads((ROOT / "pyproject.toml").read_text(encoding="utf-8")) - assert pyproject["project"]["version"] == "1.0.2" + assert pyproject["project"]["version"] == "1.0.3" def test_release_workflow_builds_and_validates_artifact_version() -> None: @@ -22,7 +22,7 @@ def test_release_workflow_builds_and_validates_artifact_version() -> None: def test_docs_workflow_examples_reference_current_release_tag() -> None: workflow = (ROOT / ".github" / "workflows" / "docs.yml").read_text(encoding="utf-8") - assert "v1.0.2" in workflow + assert "v1.0.3" in workflow def test_github_release_job_selects_repository_without_checkout() -> None: diff --git a/uv.lock b/uv.lock index 92b4e94..94656f8 100644 --- a/uv.lock +++ b/uv.lock @@ -1421,7 +1421,7 @@ wheels = [ [[package]] name = "pytest-skill-engineering" -version = "1.0.2" +version = "1.0.3" source = { editable = "." } dependencies = [ { name = "github-copilot-sdk" }, From 38171270f928fc6fc2df6756e10558688704d191 Mon Sep 17 00:00:00 2001 From: sbroenne Date: Thu, 1 Oct 2026 20:30:52 +0200 Subject: [PATCH 2/2] Remove circular imports from result contracts and MCP validation Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../copilot/config.py | 3 +- .../copilot/contracts.py | 26 +----- .../copilot/events.py | 2 +- .../copilot/result.py | 25 +++++- src/pytest_skill_engineering/core/evals.py | 2 +- src/pytest_skill_engineering/core/plugin.py | 78 +---------------- .../core/validation.py | 83 +++++++++++++++++++ tests/contracts/test_review_regressions.py | 11 ++- 8 files changed, 121 insertions(+), 109 deletions(-) create mode 100644 src/pytest_skill_engineering/core/validation.py diff --git a/src/pytest_skill_engineering/copilot/config.py b/src/pytest_skill_engineering/copilot/config.py index dc56c9a..5f91448 100644 --- a/src/pytest_skill_engineering/copilot/config.py +++ b/src/pytest_skill_engineering/copilot/config.py @@ -10,7 +10,8 @@ from pathlib import Path from typing import TYPE_CHECKING, Any -from pytest_skill_engineering.core.plugin import _read_json, _validate_mcp_servers +from pytest_skill_engineering.core.plugin import _read_json +from pytest_skill_engineering.core.validation import _validate_mcp_servers if TYPE_CHECKING: from pytest_skill_engineering.copilot.contracts import CopilotEvalConfig diff --git a/src/pytest_skill_engineering/copilot/contracts.py b/src/pytest_skill_engineering/copilot/contracts.py index b9dc04c..f367506 100644 --- a/src/pytest_skill_engineering/copilot/contracts.py +++ b/src/pytest_skill_engineering/copilot/contracts.py @@ -3,27 +3,16 @@ from __future__ import annotations from collections.abc import Awaitable, Callable, Mapping -from dataclasses import dataclass -from typing import TYPE_CHECKING, Any, Literal, Protocol, TypeAlias +from typing import Any, Literal, Protocol, TypeAlias from copilot.session import CustomAgentConfig, MCPServerConfig, ReasoningEffort, SessionHooks -if TYPE_CHECKING: - from pytest_skill_engineering.copilot.result import CopilotResult +from pytest_skill_engineering.copilot.result import CopilotResult, CopilotResultAgent CopilotCustomAgentConfig: TypeAlias = CustomAgentConfig CopilotMCPServerConfig: TypeAlias = MCPServerConfig CopilotReasoningEffort: TypeAlias = ReasoningEffort CopilotSessionHooks: TypeAlias = SessionHooks -SubagentStatus: TypeAlias = Literal["selected", "started", "completed", "failed"] - - -class CopilotResultAgent(Protocol): - """Minimal agent surface stored on :class:`CopilotResult`.""" - - @property - def working_directory(self) -> str | None: - raise NotImplementedError class CopilotRunResult(Protocol): @@ -166,17 +155,6 @@ def apply( raise NotImplementedError -@dataclass(slots=True) -class SubagentInvocation: - """A single custom-agent dispatch observed during a run.""" - - invocation_id: str - name: str - status: SubagentStatus - duration_ms: float | None = None - result: CopilotResult | None = None - - def declared_agent_tools( agent_config: CopilotCustomAgentConfig, ) -> list[str] | None: diff --git a/src/pytest_skill_engineering/copilot/events.py b/src/pytest_skill_engineering/copilot/events.py index afe009e..502145f 100644 --- a/src/pytest_skill_engineering/copilot/events.py +++ b/src/pytest_skill_engineering/copilot/events.py @@ -64,10 +64,10 @@ from datetime import timedelta from typing import TYPE_CHECKING, Any -from pytest_skill_engineering.copilot.contracts import SubagentStatus from pytest_skill_engineering.copilot.result import ( CopilotResult, SubagentInvocation, + SubagentStatus, ToolCall, Turn, UsageInfo, diff --git a/src/pytest_skill_engineering/copilot/result.py b/src/pytest_skill_engineering/copilot/result.py index 6d9a0a3..5c70a79 100644 --- a/src/pytest_skill_engineering/copilot/result.py +++ b/src/pytest_skill_engineering/copilot/result.py @@ -8,9 +8,8 @@ from dataclasses import dataclass, field from pathlib import Path -from typing import Any +from typing import Any, Literal, Protocol, TypeAlias -from pytest_skill_engineering.copilot.contracts import CopilotResultAgent, SubagentInvocation from pytest_skill_engineering.copilot.requests import RequestAudit from pytest_skill_engineering.core.result import ( SkillDiscovery, @@ -30,6 +29,28 @@ ] +SubagentStatus: TypeAlias = Literal["selected", "started", "completed", "failed"] + + +class CopilotResultAgent(Protocol): + """Minimal agent surface stored on :class:`CopilotResult`.""" + + @property + def working_directory(self) -> str | None: + raise NotImplementedError + + +@dataclass(slots=True) +class SubagentInvocation: + """A single custom-agent dispatch observed during a run.""" + + invocation_id: str + name: str + status: SubagentStatus + duration_ms: float | None = None + result: CopilotResult | None = None + + @dataclass(slots=True) class CopilotResult: """Result of running a prompt against GitHub Copilot. diff --git a/src/pytest_skill_engineering/core/evals.py b/src/pytest_skill_engineering/core/evals.py index 91d0c84..b76ad58 100644 --- a/src/pytest_skill_engineering/core/evals.py +++ b/src/pytest_skill_engineering/core/evals.py @@ -192,7 +192,7 @@ def load_custom_agent( if key in metadata: config[key] = metadata[key] if "mcp-servers" in metadata: - from pytest_skill_engineering.core.plugin import _validate_mcp_servers + from pytest_skill_engineering.core.validation import _validate_mcp_servers config["mcp_servers"] = _validate_mcp_servers( metadata["mcp-servers"], path, field="mcp-servers" diff --git a/src/pytest_skill_engineering/core/plugin.py b/src/pytest_skill_engineering/core/plugin.py index ff1725b..269a18a 100644 --- a/src/pytest_skill_engineering/core/plugin.py +++ b/src/pytest_skill_engineering/core/plugin.py @@ -29,14 +29,13 @@ from __future__ import annotations import json -import math from dataclasses import dataclass, field from pathlib import Path from typing import Any -from urllib.parse import urlsplit from pytest_skill_engineering.core.evals import load_custom_agent, load_instruction_file from pytest_skill_engineering.core.skill import Skill +from pytest_skill_engineering.core.validation import _string_field, _validate_mcp_servers @dataclass(slots=True, frozen=True) @@ -248,23 +247,6 @@ def _append_file_content(parts: list[str], file_path: Path, *, required: bool = parts.append(load_instruction_file(file_path)["content"]) -def _string_field( - data: dict[str, Any], - field: str, - source: Path, - *, - required: bool = False, - allow_empty: bool = False, -) -> str: - """Read a string field, distinguishing optional absence from invalid content.""" - if field not in data and not required: - return "" - value = data.get(field) - if not isinstance(value, str) or (not allow_empty and not value.strip()): - raise ValueError(f"{source}: '{field}' must be a non-empty string") - return value - - def _parse_metadata(manifest: dict[str, Any], source: Path) -> PluginMetadata: """Extract metadata from plugin.json manifest.""" return PluginMetadata( @@ -359,64 +341,6 @@ def _parse_mcp_servers(manifest: dict[str, Any], source: Path) -> dict[str, dict return _validate_mcp_servers(manifest[fields[0]], source, field=fields[0]) -def _validate_mcp_servers(raw: Any, source: Path, *, field: str) -> dict[str, dict[str, Any]]: - """Validate the shared server mapping contract for plugin and MCP config files.""" - if not isinstance(raw, dict): - raise ValueError(f"{source}: '{field}' must be an object") - for name, config in raw.items(): - label = f"MCP server '{name}'" - if not isinstance(name, str) or not name.strip() or not isinstance(config, dict): - raise ValueError(f"{source}: {label} must have a non-empty name and object config") - transport = config.get("type") - if "type" in config and transport not in ("stdio", "local", "http", "sse"): - raise ValueError(f"{source}: {label} has unsupported transport type {transport!r}") - has_command = "command" in config - has_url = "url" in config - if has_command == has_url: - raise ValueError(f"{source}: {label} requires exactly one of 'command' or 'url'") - if ( - transport in ("http", "sse") - and not has_url - or (transport in ("stdio", "local") and not has_command) - ): - raise ValueError(f"{source}: {label} transport does not match command/url") - for key in ("command", "url", "cwd"): - if key in config: - _string_field(config, key, source, required=True) - if has_url: - try: - url = urlsplit(config["url"]) - if url.scheme not in ("http", "https") or not url.hostname: - raise ValueError("expected an absolute HTTP(S) URL") - _ = url.port - except ValueError as exc: - raise ValueError(f"{source}: {label} invalid 'url': {exc}") from exc - for key in ("args", "tools"): - if key in config and ( - not isinstance(config[key], list) - or any(not isinstance(item, str) for item in config[key]) - ): - raise ValueError(f"{source}: {label} '{key}' must be a list of strings") - if "tools" in config and any(not tool.strip() for tool in config["tools"]): - raise ValueError(f"{source}: {label} 'tools' entries must be non-empty strings") - for key in ("env", "headers"): - if key in config and ( - not isinstance(config[key], dict) - or any( - not isinstance(k, str) or not isinstance(v, str) for k, v in config[key].items() - ) - ): - raise ValueError(f"{source}: {label} '{key}' must be a string-to-string object") - if "timeout" in config and ( - isinstance(config["timeout"], bool) - or not isinstance(config["timeout"], (int, float)) - or not math.isfinite(config["timeout"]) - or config["timeout"] <= 0 - ): - raise ValueError(f"{source}: {label} 'timeout' must be a positive number") - return dict(raw) - - def _parse_hooks(plugin_dir: Path, manifest: dict[str, Any]) -> list[HookDefinition]: """Parse hooks from hooks.json or plugin.json hooks field.""" hooks_file = plugin_dir / "hooks.json" diff --git a/src/pytest_skill_engineering/core/validation.py b/src/pytest_skill_engineering/core/validation.py new file mode 100644 index 0000000..8afebe3 --- /dev/null +++ b/src/pytest_skill_engineering/core/validation.py @@ -0,0 +1,83 @@ +"""Shared validation for plugin, custom-agent, and MCP configuration fields.""" + +from __future__ import annotations + +import math +from pathlib import Path +from typing import Any +from urllib.parse import urlsplit + + +def _string_field( + data: dict[str, Any], + field: str, + source: Path, + *, + required: bool = False, + allow_empty: bool = False, +) -> str: + """Read a string field, distinguishing optional absence from invalid content.""" + if field not in data and not required: + return "" + value = data.get(field) + if not isinstance(value, str) or (not allow_empty and not value.strip()): + raise ValueError(f"{source}: '{field}' must be a non-empty string") + return value + + +def _validate_mcp_servers(raw: Any, source: Path, *, field: str) -> dict[str, dict[str, Any]]: + """Validate the shared server mapping contract for plugin and MCP config files.""" + if not isinstance(raw, dict): + raise ValueError(f"{source}: '{field}' must be an object") + for name, config in raw.items(): + label = f"MCP server '{name}'" + if not isinstance(name, str) or not name.strip() or not isinstance(config, dict): + raise ValueError(f"{source}: {label} must have a non-empty name and object config") + transport = config.get("type") + if "type" in config and transport not in ("stdio", "local", "http", "sse"): + raise ValueError(f"{source}: {label} has unsupported transport type {transport!r}") + has_command = "command" in config + has_url = "url" in config + if has_command == has_url: + raise ValueError(f"{source}: {label} requires exactly one of 'command' or 'url'") + if ( + transport in ("http", "sse") + and not has_url + or (transport in ("stdio", "local") and not has_command) + ): + raise ValueError(f"{source}: {label} transport does not match command/url") + for key in ("command", "url", "cwd"): + if key in config: + _string_field(config, key, source, required=True) + if has_url: + try: + url = urlsplit(config["url"]) + if url.scheme not in ("http", "https") or not url.hostname: + raise ValueError("expected an absolute HTTP(S) URL") + _ = url.port + except ValueError as exc: + raise ValueError(f"{source}: {label} invalid 'url': {exc}") from exc + for key in ("args", "tools"): + if key in config and ( + not isinstance(config[key], list) + or any(not isinstance(item, str) for item in config[key]) + ): + raise ValueError(f"{source}: {label} '{key}' must be a list of strings") + if "tools" in config and any(not tool.strip() for tool in config["tools"]): + raise ValueError(f"{source}: {label} 'tools' entries must be non-empty strings") + for key in ("env", "headers"): + if key in config and ( + not isinstance(config[key], dict) + or any( + not isinstance(k, str) or not isinstance(v, str) for k, v in config[key].items() + ) + ): + raise ValueError(f"{source}: {label} '{key}' must be a string-to-string object") + if "timeout" in config and ( + isinstance(config["timeout"], bool) + or not isinstance(config["timeout"], (int, float)) + or not math.isfinite(config["timeout"]) + or config["timeout"] <= 0 + ): + raise ValueError(f"{source}: {label} 'timeout' must be a positive number") + return dict(raw) diff --git a/tests/contracts/test_review_regressions.py b/tests/contracts/test_review_regressions.py index b7f284d..39fce1e 100644 --- a/tests/contracts/test_review_regressions.py +++ b/tests/contracts/test_review_regressions.py @@ -8,7 +8,7 @@ import sys from inspect import isawaitable from pathlib import Path -from typing import Any +from typing import Any, get_type_hints from uuid import uuid4 import pytest @@ -16,7 +16,7 @@ from copilot.tools import ToolInvocation from pytest_skill_engineering.copilot.config import snapshot_session_configuration -from pytest_skill_engineering.copilot.contracts import CopilotEvalConfig, SubagentInvocation +from pytest_skill_engineering.copilot.contracts import CopilotEvalConfig from pytest_skill_engineering.copilot.eval import CopilotEval from pytest_skill_engineering.copilot.events import EventMapper from pytest_skill_engineering.copilot.fixtures import _convert_to_aitest @@ -24,7 +24,7 @@ _inject_skill_reference_tools, _make_subagent_dispatch_tool, ) -from pytest_skill_engineering.copilot.result import CopilotResult +from pytest_skill_engineering.copilot.result import CopilotResult, SubagentInvocation from pytest_skill_engineering.core.evals import load_custom_agent, load_custom_agents from pytest_skill_engineering.core.result import ToolCall, Turn, UsageInfo from pytest_skill_engineering.core.serialization import serialize_dataclass @@ -33,6 +33,11 @@ from pytest_skill_engineering.reporting.collector import TestReport as CaseReport +def test_runtime_result_type_hints_resolve() -> None: + assert get_type_hints(CopilotResult)["subagent_invocations"] == list[SubagentInvocation] + assert get_type_hints(SubagentInvocation)["result"] == CopilotResult | None + + def test_agent_settings_are_shared_across_loaders(tmp_path: Path) -> None: definition = ( "---\nname: declared-name\ndescription: Specialist\nmodel: child-model\n"