diff --git a/apps/web/src/content/docs/docs/next/graders/llm-graders.mdx b/apps/web/src/content/docs/docs/next/graders/llm-graders.mdx index 286d476fc..7ec94efe6 100644 --- a/apps/web/src/content/docs/docs/next/graders/llm-graders.mdx +++ b/apps/web/src/content/docs/docs/next/graders/llm-graders.mdx @@ -143,8 +143,8 @@ assert: prompt: ./prompts/pass-fail.md ``` -Each `target:` value must match a grader `id` from `.agentv/config.yaml`, an -eval-local `graders` entry, or a directly referenced `graders: file://...` field. +Each `target:` value must match a target `id` from `.agentv/config.yaml` or +from an eval-local `targets` entry. ### TypeScript Template diff --git a/packages/core/src/evaluation/graders/index.ts b/packages/core/src/evaluation/graders/index.ts index 81874f7de..37ec34050 100644 --- a/packages/core/src/evaluation/graders/index.ts +++ b/packages/core/src/evaluation/graders/index.ts @@ -54,7 +54,6 @@ export type { LlmGraderOptions } from './llm-grader.js'; export { formatToolCalls } from './format-tool-calls.js'; -export { SkillTriggerGrader } from './skill-trigger.js'; export { SkillUsedGrader } from './skill-used.js'; export { assembleLlmGraderPrompt } from './llm-grader-prompt.js'; diff --git a/packages/core/src/evaluation/graders/skill-trigger.ts b/packages/core/src/evaluation/graders/skill-trigger.ts deleted file mode 100644 index cd603d5f6..000000000 --- a/packages/core/src/evaluation/graders/skill-trigger.ts +++ /dev/null @@ -1,109 +0,0 @@ -/** - * Built-in skill-trigger evaluator. - * - * Detects whether the agent invoked a named skill during a session. - * Works with canonical tool names produced by normalizeToolCall() — no - * provider-specific matching logic needed. - * - * Detection logic: - * - Scans ALL tool calls (not just the first) for skill invocation evidence. - * - Skill tool: checks `tool` (case-insensitive) === 'skill' and `input.skill` contains the skill name. - * - Read tool: checks `tool` (case-insensitive) === 'read' and `input.file_path` or `input.path` contains a skills/ path. - * - Fallback: checks tool output for skill file path references. - * - Supports negative cases via should_trigger: false. - * - * Prerequisites: - * All providers and import parsers must call normalizeToolCall() when - * constructing ToolCall objects. This ensures canonical tool names - * ("Skill", "Read", "Write", "Edit", "Bash") and canonical input field - * names (input.skill, input.file_path) regardless of provider. - */ - -import type { SkillTriggerGraderConfig } from '../types.js'; -import type { EvaluationContext, EvaluationScore, Grader } from './types.js'; - -export class SkillTriggerGrader implements Grader { - readonly kind = 'skill-trigger'; - - private readonly config: SkillTriggerGraderConfig; - - constructor(config: SkillTriggerGraderConfig) { - this.config = config; - } - - evaluate(context: EvaluationContext): EvaluationScore { - const skillName = this.config.skill; - const shouldTrigger = this.config.should_trigger !== false; - - const allToolCalls = (context.output ?? []).flatMap((msg) => msg.toolCalls ?? []); - - let triggered = false; - let evidence = ''; - - for (const toolCall of allToolCalls) { - const toolName = toolCall.tool ?? ''; - const input = (toolCall.input ?? {}) as Record; - - if (toolName.toLowerCase() === 'skill') { - const skillArg = String(input.skill ?? ''); - if (skillArg.includes(skillName)) { - triggered = true; - evidence = `Skill tool invoked with skill="${skillArg}"`; - break; - } - } else if (toolName.toLowerCase() === 'read') { - const filePath = String(input.file_path ?? input.path ?? ''); - if (filePath.includes(`skills/${skillName}/`)) { - triggered = true; - evidence = `Read tool loaded skill file: ${filePath}`; - break; - } - } - - // Fallback: check if a tool's output contains a skill file path. - if (!triggered && toolCall.output != null) { - const outputStr = - typeof toolCall.output === 'string' ? toolCall.output : JSON.stringify(toolCall.output); - if (outputStr.includes(`skills/${skillName}/`)) { - triggered = true; - evidence = `Tool "${toolName}" output referenced skill file for "${skillName}"`; - break; - } - } - } - - const pass = triggered === shouldTrigger; - - if (pass) { - return { - score: 1, - verdict: 'pass', - assertions: [ - { - text: shouldTrigger - ? evidence || `Skill "${skillName}" triggered as expected` - : `Skill "${skillName}" correctly did not trigger`, - passed: true, - }, - ], - expectedAspectCount: 1, - }; - } - - return { - score: 0, - verdict: 'fail', - assertions: [ - { - text: shouldTrigger - ? allToolCalls.length > 0 - ? `Skill "${skillName}" not found in ${allToolCalls.length} tool call(s)` - : 'No tool calls recorded' - : evidence || `Skill "${skillName}" triggered unexpectedly`, - passed: false, - }, - ], - expectedAspectCount: 1, - }; - } -} diff --git a/packages/core/src/evaluation/loaders/eval-yaml-transpiler.ts b/packages/core/src/evaluation/loaders/eval-yaml-transpiler.ts index e64ef6ba1..a404d4799 100644 --- a/packages/core/src/evaluation/loaders/eval-yaml-transpiler.ts +++ b/packages/core/src/evaluation/loaders/eval-yaml-transpiler.ts @@ -114,7 +114,11 @@ function assertionToNaturalLanguage(entry: RawAssertEntry): string | null { switch (type) { case 'skill-trigger': - // Handled separately — not an NL assertion + throw new Error(staleSkillTriggerMessage(entry)); + + case 'skill-used': + case 'not-skill-used': + // Handled separately as Agent Skills trigger labels. return null; case 'llm-rubric': @@ -249,12 +253,46 @@ function assertionToNaturalLanguageList(entry: RawAssertEntry): string[] { return nl !== null ? [nl] : []; } -/** - * Extract skill-trigger entries from an assertion list. - * Returns entries with type === 'skill-trigger'. - */ -function extractTriggerAssertions(assertions: RawAssertEntry[]): RawAssertEntry[] { - return assertions.filter((a) => a.type === 'skill-trigger'); +function staleSkillTriggerMessage(entry: RawAssertEntry): string { + const skill = typeof entry.skill === 'string' ? entry.skill.trim() : ''; + const shouldTrigger = entry.should_trigger !== false; + if (!skill) { + return "Authored assertion type 'skill-trigger' has been removed. Use 'skill-used' with value: for expected skill use, or 'not-skill-used' with value: when the skill must not be used."; + } + const replacementType = shouldTrigger ? 'skill-used' : 'not-skill-used'; + return `Authored assertion type 'skill-trigger' has been removed. Replace skill: ${skill} with type: ${replacementType}, value: ${skill}.`; +} + +interface SkillUseAssertion { + readonly skill: string; + readonly shouldTrigger: boolean; +} + +function skillNameFromValue(value: unknown): string | undefined { + if (typeof value === 'string' && value.trim()) { + return value.trim(); + } + if (value && typeof value === 'object' && !Array.isArray(value)) { + const name = (value as Record).name; + return typeof name === 'string' && name.trim() ? name.trim() : undefined; + } + return undefined; +} + +function extractSkillUseAssertions(assertions: RawAssertEntry[]): SkillUseAssertion[] { + return assertions.flatMap((entry) => { + if (entry.type === 'skill-trigger') { + throw new Error(staleSkillTriggerMessage(entry)); + } + if (entry.type !== 'skill-used' && entry.type !== 'not-skill-used') { + return []; + } + const skill = skillNameFromValue(entry.value); + if (!skill) { + return []; + } + return [{ skill, shouldTrigger: entry.type === 'skill-used' }]; + }); } // --------------------------------------------------------------------------- @@ -363,9 +401,7 @@ export function transpileEvalYaml(suite: unknown, source = 'EVAL.yaml'): Transpi const suiteAssertions = rawSuite.assert ?? []; // Suite-level NL assertions (appended to every test) - const suiteNlAssertions: string[] = suiteAssertions - .filter((a) => a.type !== 'skill-trigger') - .flatMap(assertionToNaturalLanguageList); + const suiteNlAssertions: string[] = suiteAssertions.flatMap(assertionToNaturalLanguageList); /** * Helper: get or create the EvalsJsonFile for a skill. @@ -394,7 +430,7 @@ export function transpileEvalYaml(suite: unknown, source = 'EVAL.yaml'): Transpi ); } - // Collect NL assertions (not skill-trigger) + // Collect NL assertions (not skill-use assertions) const nlAssertions: string[] = []; // Prepend test-level criteria as NL assertion @@ -403,7 +439,7 @@ export function transpileEvalYaml(suite: unknown, source = 'EVAL.yaml'): Transpi } for (const entry of caseAssertions) { - if (entry.type !== 'skill-trigger') { + if (entry.type !== 'skill-used' && entry.type !== 'not-skill-used') { nlAssertions.push(...assertionToNaturalLanguageList(entry)); } } @@ -411,7 +447,7 @@ export function transpileEvalYaml(suite: unknown, source = 'EVAL.yaml'): Transpi // Append suite-level NL assertions nlAssertions.push(...suiteNlAssertions); - const triggerJudges = extractTriggerAssertions(caseAssertions); + const triggerJudges = extractSkillUseAssertions(caseAssertions); const { prompt, files: inputFiles } = extractInput(rawCase); const expectedOutput = extractExpectedOutput(rawCase.expected_output); @@ -428,7 +464,7 @@ export function transpileEvalYaml(suite: unknown, source = 'EVAL.yaml'): Transpi }; if (triggerJudges.length === 0) { - // No skill-trigger: place in dominant skill (or _no-skill) + // No skill-use assertion: place in dominant skill (or _no-skill) // Determine dominant skill by scanning all tests (first occurrence wins) // We defer this: record with a sentinel and resolve after all tests are processed. // For now, push to _no-skill; we'll re-assign at the end. @@ -437,10 +473,8 @@ export function transpileEvalYaml(suite: unknown, source = 'EVAL.yaml'): Transpi } else { // Place in each skill with the correct should_trigger value for (const tj of triggerJudges) { - const skillName = typeof tj.skill === 'string' ? tj.skill : '_no-skill'; - const shouldTrigger = tj.should_trigger !== false; // default true - const skillFile = getSkillFile(skillName); - skillFile.evals.push({ ...baseCase, should_trigger: shouldTrigger }); + const skillFile = getSkillFile(tj.skill); + skillFile.evals.push({ ...baseCase, should_trigger: tj.shouldTrigger }); } } } diff --git a/packages/core/src/evaluation/loaders/grader-parser.ts b/packages/core/src/evaluation/loaders/grader-parser.ts index 5c62c3e76..b60a50adc 100644 --- a/packages/core/src/evaluation/loaders/grader-parser.ts +++ b/packages/core/src/evaluation/loaders/grader-parser.ts @@ -1390,34 +1390,6 @@ async function parseGraderList( continue; } - if (typeValue === 'skill-trigger') { - const skillName = asString(rawEvaluator.skill); - if (!skillName) { - logWarning(`Skipping skill-trigger evaluator '${name}' in '${evalId}': missing skill`); - continue; - } - const rawShouldTrigger = rawEvaluator.should_trigger; - const shouldTrigger = typeof rawShouldTrigger === 'boolean' ? rawShouldTrigger : undefined; - const weight = validateWeight(rawEvaluator.weight, name, evalId); - const { required, min_score } = parseRequiredAndMinScore( - rawEvaluator.required, - (rawEvaluator as Record).min_score as JsonValue | undefined, - name, - evalId, - ); - pushEvaluator({ - name, - type: 'skill-trigger', - skill: skillName, - ...(shouldTrigger !== undefined ? { should_trigger: shouldTrigger } : {}), - ...(weight !== undefined ? { weight } : {}), - ...(required !== undefined ? { required } : {}), - ...(min_score !== undefined ? { min_score } : {}), - ...(negate !== undefined ? { negate } : {}), - }); - continue; - } - if (typeValue === 'javascript' || typeValue === 'python' || typeValue === 'webhook') { const value = asString(rawEvaluator.value); if (!value || value.trim().length === 0) { @@ -2073,10 +2045,6 @@ function generateAssertionName(typeValue: string, rawEvaluator: JsonObject): str } return typeValue; } - case 'skill-trigger': { - const skillValue = asString(rawEvaluator.skill); - return skillValue ? `skill-trigger-${skillValue}` : 'skill-trigger'; - } case 'contains': return value ? `contains-${value}` : 'contains'; case 'contains-any': diff --git a/packages/core/src/evaluation/providers/cli.ts b/packages/core/src/evaluation/providers/cli.ts index 75d602466..d807dfc57 100644 --- a/packages/core/src/evaluation/providers/cli.ts +++ b/packages/core/src/evaluation/providers/cli.ts @@ -13,6 +13,7 @@ import { type TargetRuntimeConfig, runDockerSandboxCommand, } from './sandbox-runner.js'; +import { deriveSkillCallMetadataFromMessages } from './skill-calls.js'; import { buildTargetExecutionEnvelope, captureTargetExecutionLog } from './target-execution.js'; import type { CliResolvedConfig } from './targets.js'; import type { @@ -543,6 +544,7 @@ export class CliProvider implements Provider { return { output: parsed.output, + metadata: deriveSkillCallMetadataFromMessages(parsed.output), tokenUsage: parsed.tokenUsage, costUsd: parsed.costUsd, durationMs: parsed.durationMs ?? measuredDurationMs, @@ -804,6 +806,7 @@ export class CliProvider implements Provider { return { output: parsed.output, + metadata: deriveSkillCallMetadataFromMessages(parsed.output), tokenUsage: parsed.tokenUsage, costUsd: parsed.costUsd, durationMs: parsed.durationMs ?? perRequestFallbackMs, diff --git a/packages/core/src/evaluation/providers/copilot-cli.ts b/packages/core/src/evaluation/providers/copilot-cli.ts index 1d0f7aa55..495527bbf 100644 --- a/packages/core/src/evaluation/providers/copilot-cli.ts +++ b/packages/core/src/evaluation/providers/copilot-cli.ts @@ -23,6 +23,7 @@ import { import { resolveDefaultProviderLogDir } from './log-directory.js'; import { normalizeToolCall } from './normalize-tool-call.js'; import { buildPromptDocument, normalizeInputFiles } from './preread.js'; +import { deriveSkillCallMetadataFromMessages } from './skill-calls.js'; import { buildTargetExecutionEnvelope } from './target-execution.js'; import type { CopilotCliResolvedConfig, CopilotCustomProviderConfig } from './targets.js'; import type { @@ -369,6 +370,7 @@ export class CopilotCliProvider implements Provider { logFile: logger?.filePath, }, output: outputMessages, + metadata: deriveSkillCallMetadataFromMessages(outputMessages), tokenUsage, costUsd, durationMs, diff --git a/packages/core/src/evaluation/providers/copilot-sdk.ts b/packages/core/src/evaluation/providers/copilot-sdk.ts index 761be9ebc..74e5ffa84 100644 --- a/packages/core/src/evaluation/providers/copilot-sdk.ts +++ b/packages/core/src/evaluation/providers/copilot-sdk.ts @@ -15,6 +15,7 @@ import { import { resolveDefaultProviderLogDir } from './log-directory.js'; import { normalizeToolCall } from './normalize-tool-call.js'; import { buildPromptDocument, normalizeInputFiles } from './preread.js'; +import { deriveSkillCallMetadataFromMessages } from './skill-calls.js'; import type { CopilotSdkResolvedConfig } from './targets.js'; import type { Message, @@ -331,6 +332,7 @@ export class CopilotSdkProvider implements Provider { logFile: logger?.filePath, }, output, + metadata: deriveSkillCallMetadataFromMessages(output), tokenUsage, costUsd, durationMs, diff --git a/packages/core/src/evaluation/providers/pi-cli.ts b/packages/core/src/evaluation/providers/pi-cli.ts index b5d2955e3..0a7af6d72 100644 --- a/packages/core/src/evaluation/providers/pi-cli.ts +++ b/packages/core/src/evaluation/providers/pi-cli.ts @@ -35,6 +35,7 @@ import { } from './pi-provider-aliases.js'; import { extractPiTextContent, toFiniteNumber } from './pi-utils.js'; import { normalizeInputFiles } from './preread.js'; +import { deriveSkillCallMetadataFromMessages } from './skill-calls.js'; import type { PiCliResolvedConfig } from './targets.js'; import type { Message, @@ -155,6 +156,7 @@ export class PiCliProvider implements Provider { logFile: logger?.filePath, }, output, + metadata: deriveSkillCallMetadataFromMessages(output), tokenUsage, durationMs, startTime, @@ -771,7 +773,7 @@ function extractMessages(events: unknown[]): readonly Message[] { // Pi CLI may emit tool_execution_start/tool_execution_end events whose tool // calls are absent from the final agent_end messages. Reconstruct them and - // inject into the last assistant message so evaluators (e.g. skill-trigger) + // inject into the last assistant message so trajectory and skill-use graders // can detect them. const eventToolCalls = extractToolCallsFromEvents(events); if (eventToolCalls.length > 0) { diff --git a/packages/core/src/evaluation/providers/pi-coding-agent.ts b/packages/core/src/evaluation/providers/pi-coding-agent.ts index 78d238f21..2de4ae3dc 100644 --- a/packages/core/src/evaluation/providers/pi-coding-agent.ts +++ b/packages/core/src/evaluation/providers/pi-coding-agent.ts @@ -28,6 +28,7 @@ import { } from './pi-provider-aliases.js'; import { extractPiTextContent, toFiniteNumber, toPiContentArray } from './pi-utils.js'; import { normalizeInputFiles } from './preread.js'; +import { deriveSkillCallMetadataFromMessages } from './skill-calls.js'; import type { PiCodingAgentResolvedConfig } from './targets.js'; import type { Message, @@ -512,6 +513,7 @@ export class PiCodingAgentProvider implements Provider { provider: this.config.subprovider, }, output, + metadata: deriveSkillCallMetadataFromMessages(output), tokenUsage, costUsd, durationMs, diff --git a/packages/core/src/evaluation/providers/pi-rpc.ts b/packages/core/src/evaluation/providers/pi-rpc.ts index 7061e3284..3e60bb3e6 100644 --- a/packages/core/src/evaluation/providers/pi-rpc.ts +++ b/packages/core/src/evaluation/providers/pi-rpc.ts @@ -18,6 +18,7 @@ import { } from './pi-provider-aliases.js'; import { extractPiTextContent, toFiniteNumber } from './pi-utils.js'; import { normalizeInputFiles } from './preread.js'; +import { deriveSkillCallMetadataFromMessages } from './skill-calls.js'; import type { PiRpcResolvedConfig } from './targets.js'; import type { Message, @@ -137,6 +138,7 @@ export class PiRpcProvider implements Provider { inputFiles, }, output, + metadata: deriveSkillCallMetadataFromMessages(output), tokenUsage, durationMs: endedAt - startedAt, startTime: new Date(startedAt).toISOString(), diff --git a/packages/core/src/evaluation/providers/sdk-child-protocol.ts b/packages/core/src/evaluation/providers/sdk-child-protocol.ts index b010bbd61..8d9d79e1c 100644 --- a/packages/core/src/evaluation/providers/sdk-child-protocol.ts +++ b/packages/core/src/evaluation/providers/sdk-child-protocol.ts @@ -1,4 +1,5 @@ import type { JsonObject } from '../types.js'; +import { deriveSkillCallMetadataFromMessages } from './skill-calls.js'; import type { ChatPrompt, Message, @@ -156,10 +157,12 @@ export function providerResponseToWire(response: ProviderResponse): SdkChildProv } export function providerResponseFromWire(response: SdkChildProviderResponseWire): ProviderResponse { + const derivedMetadata = deriveSkillCallMetadataFromMessages(response.output); + const metadata = { ...derivedMetadata, ...response.metadata }; return { raw: response.raw, usage: response.usage, - metadata: response.metadata, + metadata: Object.keys(metadata).length > 0 ? metadata : undefined, output: response.output, tokenUsage: response.token_usage, costUsd: response.cost_usd, diff --git a/packages/core/src/evaluation/providers/skill-calls.ts b/packages/core/src/evaluation/providers/skill-calls.ts index e503f8b2a..2d6d578f1 100644 --- a/packages/core/src/evaluation/providers/skill-calls.ts +++ b/packages/core/src/evaluation/providers/skill-calls.ts @@ -12,6 +12,12 @@ export function deriveSkillCallsFromMessages( return deriveSkillCallsFromToolCalls(messages.flatMap((message) => message.toolCalls ?? [])); } +export function deriveSkillCallMetadataFromMessages( + messages: readonly Message[] | undefined, +): ProviderResponseSkillMetadata | undefined { + return skillCallMetadata(deriveSkillCallsFromMessages(messages)); +} + export function deriveSkillCallsFromToolCalls( toolCalls: readonly ToolCall[] | undefined, ): readonly SkillCall[] { @@ -105,11 +111,16 @@ export function skillCallMetadata( if (skillCalls.length === 0) { return undefined; } - return { skillCalls }; + const confirmed = skillCalls.filter((skillCall) => skillCall.isError !== true); + return dropUndefined({ + skillCalls: confirmed.length > 0 ? confirmed : undefined, + attemptedSkillCalls: confirmed.length < skillCalls.length ? skillCalls : undefined, + }); } type ProviderResponseSkillMetadata = { - readonly skillCalls: readonly SkillCall[]; + readonly skillCalls?: readonly SkillCall[]; + readonly attemptedSkillCalls?: readonly SkillCall[]; }; function skillPathCandidates(text: string): Array<{ name: string; path: string }> { diff --git a/packages/core/src/evaluation/registry/builtin-graders.ts b/packages/core/src/evaluation/registry/builtin-graders.ts index 765c078c9..a3df32524 100644 --- a/packages/core/src/evaluation/registry/builtin-graders.ts +++ b/packages/core/src/evaluation/registry/builtin-graders.ts @@ -14,7 +14,6 @@ import { LatencyGrader, LlmGrader, ScriptGrader, - SkillTriggerGrader, SkillUsedGrader, TokenUsageGrader, ToolTrajectoryGrader, @@ -65,7 +64,6 @@ import type { ScriptAssertionGraderConfig, ScriptGraderConfig, SimilarGraderConfig, - SkillTriggerGraderConfig, SkillUsedGraderConfig, StartsWithGraderConfig, TokenUsageGraderConfig, @@ -262,11 +260,6 @@ export const executionMetricsFactory: GraderFactoryFn = (config) => { }); }; -/** Factory for `skill-trigger` evaluator. */ -export const skillTriggerFactory: GraderFactoryFn = (config) => { - return new SkillTriggerGrader(config as SkillTriggerGraderConfig); -}; - /** Factory for Promptfoo-compatible `skill-used` and `not-skill-used` evaluators. */ export const skillUsedFactory: GraderFactoryFn = (config) => { return new SkillUsedGrader(config as SkillUsedGraderConfig); @@ -448,7 +441,6 @@ export function createBuiltinRegistry(): GraderRegistry { .register('execution-metrics', executionMetricsFactory) .register('skill-used', skillUsedFactory) .register('not-skill-used', skillUsedFactory) - .register('skill-trigger', skillTriggerFactory) .register('assert-set', assertSetFactory) .register('contains', containsFactory) .register('contains-any', containsAnyFactory) diff --git a/packages/core/src/evaluation/types.ts b/packages/core/src/evaluation/types.ts index 19be9a8af..bc8755b78 100644 --- a/packages/core/src/evaluation/types.ts +++ b/packages/core/src/evaluation/types.ts @@ -192,7 +192,6 @@ const GRADER_KIND_VALUES = [ 'cost', 'token-usage', 'execution-metrics', - 'skill-trigger', 'assert-set', 'llm-rubric', 'contains', @@ -893,25 +892,6 @@ export type AssertSetGraderConfig = { readonly negate?: boolean; }; -/** - * @deprecated Internal compatibility config for older runtime helpers. - * Authored eval YAML rejects `skill-trigger`; use `skill-used` or - * `not-skill-used` instead. - */ -export type SkillTriggerGraderConfig = { - readonly name: string; - readonly type: 'skill-trigger'; - /** The skill name to check for (case-sensitive substring match) */ - readonly skill: string; - /** Whether the skill is expected to trigger (default: true) */ - readonly should_trigger?: boolean; - readonly weight?: number; - readonly required?: boolean; - /** Minimum score (0-1) for this evaluator to pass. Independent of `required` gate. */ - readonly min_score?: number; - readonly negate?: boolean; -}; - export type SkillUsedValue = | string | readonly string[] @@ -982,7 +962,6 @@ export type GraderConfig = ( | CostGraderConfig | TokenUsageGraderConfig | ExecutionMetricsGraderConfig - | SkillTriggerGraderConfig | TrajectoryGraderConfig | ContainsGraderConfig | ContainsAnyGraderConfig diff --git a/packages/core/test/evaluation/graders/promptfoo-assertions.test.ts b/packages/core/test/evaluation/graders/promptfoo-assertions.test.ts index 0ee224c51..484122105 100644 --- a/packages/core/test/evaluation/graders/promptfoo-assertions.test.ts +++ b/packages/core/test/evaluation/graders/promptfoo-assertions.test.ts @@ -54,6 +54,25 @@ describe('promptfoo-compatible built-in assertions', () => { } }); + it('does not register removed skill-trigger assertions', async () => { + const registry = createBuiltinRegistry(); + + await expect( + registry.create( + { name: 'stale', type: 'skill-trigger', skill: 'csv-analyzer' } as unknown as GraderConfig, + { + llmGrader: { + kind: 'llm-grader', + evaluate() { + throw new Error('not used'); + }, + }, + registry, + }, + ), + ).rejects.toThrow('Unknown grader type: "skill-trigger"'); + }); + it('runs javascript assertions in-process', async () => { const result = await run({ name: 'js', diff --git a/packages/core/test/evaluation/graders/skill-trigger.test.ts b/packages/core/test/evaluation/graders/skill-trigger.test.ts deleted file mode 100644 index 9519a8cac..000000000 --- a/packages/core/test/evaluation/graders/skill-trigger.test.ts +++ /dev/null @@ -1,377 +0,0 @@ -import { describe, expect, it } from 'vitest'; -import { SkillTriggerGrader } from '../../../src/evaluation/graders/skill-trigger.js'; -import type { EvaluationContext } from '../../../src/evaluation/graders/types.js'; -import type { SkillTriggerGraderConfig } from '../../../src/evaluation/types.js'; - -// biome-ignore lint/suspicious/noExplicitAny: test helper with partial context -function makeContext(overrides: Record = {}): EvaluationContext { - return { - evalCase: { id: 'test', input: 'test input' }, - candidate: 'test output', - target: { name: 'test-target' }, - provider: { kind: 'claude-cli', targetName: 'test' }, - attempt: 1, - promptInputs: { question: 'test' }, - now: new Date(), - ...overrides, - // biome-ignore lint/suspicious/noExplicitAny: partial context for tests - } as any; -} - -function makeConfig(overrides: Partial = {}): SkillTriggerGraderConfig { - return { - name: 'test-trigger', - type: 'skill-trigger', - skill: 'csv-analyzer', - ...overrides, - }; -} - -describe('SkillTriggerGrader', () => { - describe('canonical tool names (provider-agnostic)', () => { - it('should detect Skill tool with matching skill name', () => { - const evaluator = new SkillTriggerGrader(makeConfig()); - const context = makeContext({ - output: [ - { - role: 'assistant', - content: '', - toolCalls: [{ tool: 'Skill', input: { skill: 'csv-analyzer' } }], - }, - ], - }); - const result = evaluator.evaluate(context); - expect(result.verdict).toBe('pass'); - expect(result.score).toBe(1); - }); - - it('should detect Read tool loading skill file via file_path', () => { - const evaluator = new SkillTriggerGrader(makeConfig()); - const context = makeContext({ - output: [ - { - role: 'assistant', - content: '', - toolCalls: [ - { - tool: 'Read', - input: { file_path: '/path/to/skills/csv-analyzer/SKILL.md' }, - }, - ], - }, - ], - }); - const result = evaluator.evaluate(context); - expect(result.verdict).toBe('pass'); - expect(result.score).toBe(1); - }); - - it('should detect skill via tool output reference', () => { - const evaluator = new SkillTriggerGrader(makeConfig()); - const context = makeContext({ - output: [ - { - role: 'assistant', - content: '', - toolCalls: [ - { - tool: 'Bash', - input: { command: 'grep -r skill' }, - output: 'Found: .agents/skills/csv-analyzer/SKILL.md', - }, - ], - }, - ], - }); - const result = evaluator.evaluate(context); - expect(result.verdict).toBe('pass'); - }); - - it('should fail when skill name does not match', () => { - const evaluator = new SkillTriggerGrader(makeConfig()); - const context = makeContext({ - output: [ - { - role: 'assistant', - content: '', - toolCalls: [{ tool: 'Skill', input: { skill: 'other-skill' } }], - }, - ], - }); - const result = evaluator.evaluate(context); - expect(result.verdict).toBe('fail'); - }); - - it('should fail when Read loads non-skill file', () => { - const evaluator = new SkillTriggerGrader(makeConfig()); - const context = makeContext({ - output: [ - { - role: 'assistant', - content: '', - toolCalls: [{ tool: 'Read', input: { file_path: '/workspace/README.md' } }], - }, - ], - }); - const result = evaluator.evaluate(context); - expect(result.verdict).toBe('fail'); - }); - - it('should fail when only unrelated tools are called', () => { - const evaluator = new SkillTriggerGrader(makeConfig()); - const context = makeContext({ - output: [ - { - role: 'assistant', - content: '', - toolCalls: [{ tool: 'Bash', input: { command: 'ls' } }], - }, - ], - }); - const result = evaluator.evaluate(context); - expect(result.verdict).toBe('fail'); - }); - - it('should handle no tool calls', () => { - const evaluator = new SkillTriggerGrader(makeConfig()); - const context = makeContext({ - output: [{ role: 'assistant', content: 'no tools used' }], - }); - const result = evaluator.evaluate(context); - expect(result.verdict).toBe('fail'); - expect(result.assertions.filter((a) => !a.passed)[0].text).toBe('No tool calls recorded'); - }); - - it('should work with any provider kind (provider-agnostic)', () => { - for (const kind of ['claude-cli', 'copilot-cli', 'codex-cli', 'pi-cli', 'openai']) { - const evaluator = new SkillTriggerGrader(makeConfig()); - const context = makeContext({ - provider: { kind, targetName: 'test' }, - output: [ - { - role: 'assistant', - content: '', - toolCalls: [{ tool: 'Skill', input: { skill: 'csv-analyzer' } }], - }, - ], - }); - const result = evaluator.evaluate(context); - expect(result.verdict).toBe('pass'); - } - }); - }); - - describe('should_trigger: false', () => { - it('should pass when skill is not triggered', () => { - const evaluator = new SkillTriggerGrader(makeConfig({ should_trigger: false })); - const context = makeContext({ - output: [ - { - role: 'assistant', - content: '', - toolCalls: [{ tool: 'Bash', input: { command: 'ls' } }], - }, - ], - }); - const result = evaluator.evaluate(context); - expect(result.verdict).toBe('pass'); - }); - - it('should fail when skill is triggered unexpectedly', () => { - const evaluator = new SkillTriggerGrader(makeConfig({ should_trigger: false })); - const context = makeContext({ - output: [ - { - role: 'assistant', - content: '', - toolCalls: [{ tool: 'Skill', input: { skill: 'csv-analyzer' } }], - }, - ], - }); - const result = evaluator.evaluate(context); - expect(result.verdict).toBe('fail'); - }); - - it('should pass with no tool calls', () => { - const evaluator = new SkillTriggerGrader(makeConfig({ should_trigger: false })); - const context = makeContext({ - output: [{ role: 'assistant', content: 'no tools used' }], - }); - const result = evaluator.evaluate(context); - expect(result.verdict).toBe('pass'); - }); - }); - - describe('full transcript scanning', () => { - it('should pass when skill triggers after a preamble skill', () => { - const evaluator = new SkillTriggerGrader(makeConfig()); - const context = makeContext({ - output: [ - { - role: 'assistant', - content: '', - toolCalls: [ - { tool: 'Skill', input: { skill: 'using-superpowers' } }, - { tool: 'Skill', input: { skill: 'csv-analyzer' } }, - ], - }, - ], - }); - const result = evaluator.evaluate(context); - expect(result.verdict).toBe('pass'); - }); - - it('should pass when skill triggers in a later message', () => { - const evaluator = new SkillTriggerGrader(makeConfig()); - const context = makeContext({ - output: [ - { - role: 'assistant', - content: 'thinking...', - toolCalls: [{ tool: 'Bash', input: { command: 'ls' } }], - }, - { - role: 'assistant', - content: '', - toolCalls: [{ tool: 'Skill', input: { skill: 'csv-analyzer' } }], - }, - ], - }); - const result = evaluator.evaluate(context); - expect(result.verdict).toBe('pass'); - }); - - it('should fail when target skill never appears anywhere in transcript', () => { - const evaluator = new SkillTriggerGrader(makeConfig()); - const context = makeContext({ - output: [ - { - role: 'assistant', - content: '', - toolCalls: [ - { tool: 'Skill', input: { skill: 'using-superpowers' } }, - { tool: 'Bash', input: { command: 'ls' } }, - ], - }, - ], - }); - const result = evaluator.evaluate(context); - expect(result.verdict).toBe('fail'); - }); - - it('should pass for should_trigger:false when skill never appears', () => { - const evaluator = new SkillTriggerGrader(makeConfig({ should_trigger: false })); - const context = makeContext({ - output: [ - { - role: 'assistant', - content: '', - toolCalls: [{ tool: 'Skill', input: { skill: 'using-superpowers' } }], - }, - ], - }); - const result = evaluator.evaluate(context); - expect(result.verdict).toBe('pass'); - }); - - it('should fail for should_trigger:false when skill appears later', () => { - const evaluator = new SkillTriggerGrader(makeConfig({ should_trigger: false })); - const context = makeContext({ - output: [ - { - role: 'assistant', - content: '', - toolCalls: [ - { tool: 'Bash', input: { command: 'ls' } }, - { tool: 'Skill', input: { skill: 'csv-analyzer' } }, - ], - }, - ], - }); - const result = evaluator.evaluate(context); - expect(result.verdict).toBe('fail'); - }); - - it('should detect skill loaded via Read in .agents/skills path', () => { - const evaluator = new SkillTriggerGrader(makeConfig()); - const context = makeContext({ - output: [ - { - role: 'assistant', - content: '', - toolCalls: [ - { - tool: 'Read', - input: { file_path: '.agents/skills/csv-analyzer/SKILL.md' }, - }, - ], - }, - ], - }); - const result = evaluator.evaluate(context); - expect(result.verdict).toBe('pass'); - }); - - it('should detect skill loaded via Read in global path', () => { - const evaluator = new SkillTriggerGrader(makeConfig()); - const context = makeContext({ - output: [ - { - role: 'assistant', - content: '', - toolCalls: [ - { - tool: 'Read', - input: { file_path: '/home/user/.agents/skills/csv-analyzer/SKILL.md' }, - }, - ], - }, - ], - }); - const result = evaluator.evaluate(context); - expect(result.verdict).toBe('pass'); - }); - - it('should detect lowercase "read" tool with input.path (pi-sdk style)', () => { - const evaluator = new SkillTriggerGrader(makeConfig()); - const context = makeContext({ - output: [ - { - role: 'assistant', - content: '', - toolCalls: [ - { - tool: 'read', - input: { path: '/home/user/.agents/skills/csv-analyzer/SKILL.md' }, - }, - ], - }, - ], - }); - const result = evaluator.evaluate(context); - expect(result.verdict).toBe('pass'); - expect(result.score).toBe(1); - }); - - it('should detect uppercase "Read" with input.file_path (Claude Code style)', () => { - const evaluator = new SkillTriggerGrader(makeConfig()); - const context = makeContext({ - output: [ - { - role: 'assistant', - content: '', - toolCalls: [ - { - tool: 'Read', - input: { file_path: '.agents/skills/csv-analyzer/SKILL.md' }, - }, - ], - }, - ], - }); - const result = evaluator.evaluate(context); - expect(result.verdict).toBe('pass'); - expect(result.score).toBe(1); - }); - }); -}); diff --git a/packages/core/test/evaluation/loaders/eval-yaml-transpiler.test.ts b/packages/core/test/evaluation/loaders/eval-yaml-transpiler.test.ts index 8b1860e39..595089b30 100644 --- a/packages/core/test/evaluation/loaders/eval-yaml-transpiler.test.ts +++ b/packages/core/test/evaluation/loaders/eval-yaml-transpiler.test.ts @@ -28,7 +28,7 @@ const SINGLE_SKILL_SUITE = { expected_output: 'The top 3 months by revenue are November ($22,500), September ($20,100), and December ($19,400).', assert: [ - { type: 'skill-trigger', skill: 'csv-analyzer', should_trigger: true }, + { type: 'skill-used', value: 'csv-analyzer' }, { type: 'llm-rubric', value: 'Agent finds the top 3 months by revenue' }, { type: 'llm-rubric', value: 'Output identifies November as the highest revenue month' }, { type: 'contains', value: '$22,500' }, @@ -37,7 +37,7 @@ const SINGLE_SKILL_SUITE = { { id: 'irrelevant-query', input: 'What time is it?', - assert: [{ type: 'skill-trigger', skill: 'csv-analyzer', should_trigger: false }], + assert: [{ type: 'not-skill-used', value: 'csv-analyzer' }], }, ], }; @@ -107,20 +107,20 @@ describe('transpileEvalYaml — input extraction', () => { // Trigger-grader handling // --------------------------------------------------------------------------- -describe('transpileEvalYaml — skill-trigger', () => { - it('sets should_trigger: true for skill-trigger with should_trigger true', () => { +describe('transpileEvalYaml — skill-used', () => { + it('sets should_trigger: true for skill-used', () => { const { files } = transpileEvalYaml(SINGLE_SKILL_SUITE); const evals = files.get('csv-analyzer')?.evals; expect(evals[0].should_trigger).toBe(true); }); - it('sets should_trigger: false for skill-trigger with should_trigger false', () => { + it('sets should_trigger: false for not-skill-used', () => { const { files } = transpileEvalYaml(SINGLE_SKILL_SUITE); const evals = files.get('csv-analyzer')?.evals; expect(evals[1].should_trigger).toBe(false); }); - it('omits should_trigger when no skill-trigger in test', () => { + it('omits should_trigger when no skill-use assertion is present', () => { const suite = { tests: [ { @@ -137,14 +137,28 @@ describe('transpileEvalYaml — skill-trigger', () => { expect(allFiles[0].evals[0].should_trigger).toBeUndefined(); }); - it('skill-trigger is NOT included in assertions array', () => { + it('skill-used is NOT included in assertions array', () => { const { files } = transpileEvalYaml(SINGLE_SKILL_SUITE); const evals = files.get('csv-analyzer')?.evals; - // assertions should contain NL items, not 'skill-trigger' literal + // assertions should contain NL items, not deterministic skill-use literals. for (const a of evals[0].assertions) { - expect(a).not.toContain('skill-trigger'); + expect(a).not.toContain('skill-used'); } }); + + it('rejects stale skill-trigger assertions with migration guidance', () => { + expect(() => + transpileEvalYaml({ + tests: [ + { + id: 'stale', + input: 'Use this skill', + assert: [{ type: 'skill-trigger', skill: 'csv-analyzer', should_trigger: true }], + }, + ], + }), + ).toThrow('Replace skill: csv-analyzer with type: skill-used, value: csv-analyzer'); + }); }); // --------------------------------------------------------------------------- @@ -225,7 +239,7 @@ describe('transpileEvalYaml — NL assertions', () => { id: 't1', input: 'test', assert: [ - { type: 'skill-trigger', skill: 's', should_trigger: true }, + { type: 'skill-used', value: 's' }, { type: 'regex', value: '\\d{4}-\\d{2}-\\d{2}' }, ], }, @@ -243,7 +257,7 @@ describe('transpileEvalYaml — NL assertions', () => { id: 't1', input: 'test', assert: [ - { type: 'skill-trigger', skill: 's', should_trigger: true }, + { type: 'skill-used', value: 's' }, { type: 'equals', value: 'exact answer' }, ], }, @@ -260,10 +274,7 @@ describe('transpileEvalYaml — NL assertions', () => { { id: 't1', input: 'test', - assert: [ - { type: 'skill-trigger', skill: 's', should_trigger: true }, - { type: 'is-json' }, - ], + assert: [{ type: 'skill-used', value: 's' }, { type: 'is-json' }], }, ], }; @@ -279,7 +290,7 @@ describe('transpileEvalYaml — NL assertions', () => { id: 't1', input: 'test', assert: [ - { type: 'skill-trigger', skill: 's', should_trigger: true }, + { type: 'skill-used', value: 's' }, { type: 'llm-grader', prompt: 'The answer is clear and concise' }, ], }, @@ -297,7 +308,7 @@ describe('transpileEvalYaml — NL assertions', () => { id: 't1', input: 'test', assert: [ - { type: 'skill-trigger', skill: 's', should_trigger: true }, + { type: 'skill-used', value: 's' }, { type: 'llm-grader', rubrics: [ @@ -322,7 +333,7 @@ describe('transpileEvalYaml — NL assertions', () => { id: 't1', input: 'test', assert: [ - { type: 'skill-trigger', skill: 's', should_trigger: true }, + { type: 'skill-used', value: 's' }, { type: 'llm-grader', rubrics: [ @@ -347,7 +358,7 @@ describe('transpileEvalYaml — NL assertions', () => { id: 't1', input: 'test', assert: [ - { type: 'skill-trigger', skill: 's', should_trigger: true }, + { type: 'skill-used', value: 's' }, { type: 'tool-trajectory', expected: [{ tool: 'read_file' }, { tool: 'write_file' }], @@ -368,11 +379,11 @@ describe('transpileEvalYaml — NL assertions', () => { id: 't1', input: 'test', assert: [ - { type: 'skill-trigger', skill: 's', should_trigger: true }, + { type: 'skill-used', value: 's' }, { type: 'script', - metric: 'skill-trigger', - description: 'Checks skill was triggered', + metric: 'skill-use-check', + description: 'Checks skill was used', }, ], }, @@ -380,7 +391,7 @@ describe('transpileEvalYaml — NL assertions', () => { }; const { files } = transpileEvalYaml(suite); const evals = files.get('s')?.evals; - expect(evals[0].assertions[0]).toContain('agentv eval assert skill-trigger'); + expect(evals[0].assertions[0]).toContain('agentv eval assert skill-use-check'); }); it('converts script grader to agentv assert instruction with description', () => { @@ -390,7 +401,7 @@ describe('transpileEvalYaml — NL assertions', () => { id: 't1', input: 'test', assert: [ - { type: 'skill-trigger', skill: 's', should_trigger: true }, + { type: 'skill-used', value: 's' }, { type: 'script', metric: 'format-checker', @@ -416,7 +427,7 @@ describe('transpileEvalYaml — NL assertions', () => { id: 't1', input: 'test', assert: [ - { type: 'skill-trigger', skill: 's', should_trigger: true }, + { type: 'skill-used', value: 's' }, { type: 'script', command: ['bun', 'run', '.agentv/graders/output-validator.ts'], @@ -437,7 +448,7 @@ describe('transpileEvalYaml — NL assertions', () => { id: 't1', input: 'test', assert: [ - { type: 'skill-trigger', skill: 's', should_trigger: true }, + { type: 'skill-used', value: 's' }, { type: 'custom-validator', command: ['bun', 'run', '.agentv/graders/custom-validator.ts'], @@ -458,7 +469,7 @@ describe('transpileEvalYaml — NL assertions', () => { id: 't1', input: 'test', assert: [ - { type: 'skill-trigger', skill: 's', should_trigger: true }, + { type: 'skill-used', value: 's' }, { type: 'field-accuracy', fields: [{ path: 'invoice.total' }, { path: 'invoice.date' }], @@ -481,7 +492,7 @@ describe('transpileEvalYaml — NL assertions', () => { id: 't1', input: 'test', assert: [ - { type: 'skill-trigger', skill: 's', should_trigger: true }, + { type: 'skill-used', value: 's' }, { type: 'latency', threshold: 5000 }, ], }, @@ -499,7 +510,7 @@ describe('transpileEvalYaml — NL assertions', () => { id: 't1', input: 'test', assert: [ - { type: 'skill-trigger', skill: 's', should_trigger: true }, + { type: 'skill-used', value: 's' }, { type: 'cost', budget: 0.1 }, ], }, @@ -517,7 +528,7 @@ describe('transpileEvalYaml — NL assertions', () => { id: 't1', input: 'test', assert: [ - { type: 'skill-trigger', skill: 's', should_trigger: true }, + { type: 'skill-used', value: 's' }, { type: 'token-usage', max_total: 1000 }, ], }, @@ -535,7 +546,7 @@ describe('transpileEvalYaml — NL assertions', () => { id: 't1', input: 'test', assert: [ - { type: 'skill-trigger', skill: 's', should_trigger: true }, + { type: 'skill-used', value: 's' }, { type: 'execution-metrics', max_tool_calls: 10 }, ], }, @@ -573,7 +584,7 @@ describe('transpileEvalYaml — expected_output', () => { id: 't1', input: 'Hello', expected_output: [{ role: 'assistant', content: 'World' }], - assert: [{ type: 'skill-trigger', skill: 's', should_trigger: true }], + assert: [{ type: 'skill-used', value: 's' }], }, ], }; @@ -594,7 +605,7 @@ describe('transpileEvalYaml — input_files shorthand', () => { id: 't1', input: 'Analyze this file', input_files: ['data/file.csv', 'data/schema.json'], - assert: [{ type: 'skill-trigger', skill: 's', should_trigger: true }], + assert: [{ type: 'skill-used', value: 's' }], }, ], }; @@ -616,12 +627,12 @@ describe('transpileEvalYaml — suite-level assertions', () => { { id: 't1', input: 'first', - assert: [{ type: 'skill-trigger', skill: 's', should_trigger: true }], + assert: [{ type: 'skill-used', value: 's' }], }, { id: 't2', input: 'second', - assert: [{ type: 'skill-trigger', skill: 's', should_trigger: true }], + assert: [{ type: 'skill-used', value: 's' }], }, ], assert: [{ type: 'contains', value: 'global-check' }], @@ -644,12 +655,12 @@ describe('transpileEvalYaml — multi-skill', () => { { id: 't1', input: 'Hello', - assert: [{ type: 'skill-trigger', skill: 'skill-a', should_trigger: true }], + assert: [{ type: 'skill-used', value: 'skill-a' }], }, { id: 't2', input: 'World', - assert: [{ type: 'skill-trigger', skill: 'skill-b', should_trigger: true }], + assert: [{ type: 'skill-used', value: 'skill-b' }], }, ], }; @@ -659,15 +670,15 @@ describe('transpileEvalYaml — multi-skill', () => { expect(files.has('skill-b')).toBe(true); }); - it('places test in both files when it has skill-triggers for two skills', () => { + it('places test in both files when it has skill-use assertions for two skills', () => { const suite = { tests: [ { id: 'shared', input: 'Do something', assert: [ - { type: 'skill-trigger', skill: 'skill-a', should_trigger: true }, - { type: 'skill-trigger', skill: 'skill-b', should_trigger: false }, + { type: 'skill-used', value: 'skill-a' }, + { type: 'not-skill-used', value: 'skill-b' }, ], }, ], @@ -678,14 +689,14 @@ describe('transpileEvalYaml — multi-skill', () => { expect(files.get('skill-b')?.evals[0].should_trigger).toBe(false); }); - it('assigns tests with no skill-trigger to dominant skill', () => { + it('assigns tests with no skill-use assertion to dominant skill', () => { const suite = { tests: [ { id: 't1', input: 'Hello', assert: [ - { type: 'skill-trigger', skill: 'skill-a', should_trigger: true }, + { type: 'skill-used', value: 'skill-a' }, { type: 'contains', value: 'hi' }, ], }, @@ -752,12 +763,12 @@ describe('getOutputFilenames', () => { { id: 't1', input: 'Hello', - assert: [{ type: 'skill-trigger', skill: 'skill-a', should_trigger: true }], + assert: [{ type: 'skill-used', value: 'skill-a' }], }, { id: 't2', input: 'World', - assert: [{ type: 'skill-trigger', skill: 'skill-b', should_trigger: true }], + assert: [{ type: 'skill-used', value: 'skill-b' }], }, ], }; diff --git a/packages/core/test/evaluation/providers/cli.test.ts b/packages/core/test/evaluation/providers/cli.test.ts index 88c40803c..56ba1d43e 100644 --- a/packages/core/test/evaluation/providers/cli.test.ts +++ b/packages/core/test/evaluation/providers/cli.test.ts @@ -562,7 +562,11 @@ describe('CliProvider', () => { role: 'assistant', content: 'Response with tool calls', tool_calls: [ - { tool: 'search', input: { query: 'hello' }, output: 'result' }, + { + tool: 'Read', + input: { path: '.agents/skills/csv-analyzer/SKILL.md' }, + output: 'skill file', + }, { tool: 'analyze', input: { data: 123 } }, ], }, @@ -588,10 +592,20 @@ describe('CliProvider', () => { expect(response.output).toHaveLength(1); expect(response.output?.[0].role).toBe('assistant'); expect(response.output?.[0].toolCalls).toHaveLength(2); - expect(response.output?.[0].toolCalls?.[0].tool).toBe('search'); - expect(response.output?.[0].toolCalls?.[0].input).toEqual({ query: 'hello' }); - expect(response.output?.[0].toolCalls?.[0].output).toBe('result'); + expect(response.output?.[0].toolCalls?.[0].tool).toBe('Read'); + expect(response.output?.[0].toolCalls?.[0].input).toEqual({ + path: '.agents/skills/csv-analyzer/SKILL.md', + }); + expect(response.output?.[0].toolCalls?.[0].output).toBe('skill file'); expect(response.output?.[0].toolCalls?.[1].tool).toBe('analyze'); + expect(response.metadata?.skillCalls).toEqual([ + { + name: 'csv-analyzer', + input: { path: '.agents/skills/csv-analyzer/SKILL.md' }, + path: '.agents/skills/csv-analyzer/SKILL.md', + source: 'heuristic', + }, + ]); }); it('parses output from batch JSONL output', async () => { @@ -605,7 +619,7 @@ describe('CliProvider', () => { output: [ { role: 'assistant', - tool_calls: [{ tool: 'toolA', input: { x: 1 } }], + tool_calls: [{ tool: 'Read', input: { path: '.agents/skills/a/SKILL.md' } }], }, ], }; @@ -615,7 +629,7 @@ describe('CliProvider', () => { output: [ { role: 'assistant', - tool_calls: [{ tool: 'toolB', input: { y: 2 } }], + tool_calls: [{ tool: 'Read', input: { path: '.agents/skills/b/SKILL.md' } }], }, ], }; @@ -643,9 +657,11 @@ describe('CliProvider', () => { expect(responses).toHaveLength(2); expect(responses[0]?.output).toBeDefined(); - expect(responses[0]?.output?.[0].toolCalls?.[0].tool).toBe('toolA'); + expect(responses[0]?.output?.[0].toolCalls?.[0].tool).toBe('Read'); + expect(responses[0]?.metadata?.skillCalls?.[0]?.name).toBe('a'); expect(responses[1]?.output).toBeDefined(); - expect(responses[1]?.output?.[0].toolCalls?.[0].tool).toBe('toolB'); + expect(responses[1]?.output?.[0].toolCalls?.[0].tool).toBe('Read'); + expect(responses[1]?.metadata?.skillCalls?.[0]?.name).toBe('b'); }); it('handles messages without tool_calls', async () => { diff --git a/packages/core/test/evaluation/providers/copilot-cli.test.ts b/packages/core/test/evaluation/providers/copilot-cli.test.ts index 0aa5ca625..8ec87e802 100644 --- a/packages/core/test/evaluation/providers/copilot-cli.test.ts +++ b/packages/core/test/evaluation/providers/copilot-cli.test.ts @@ -186,6 +186,19 @@ describe('CopilotCliProvider custom provider ACP mode', () => { const runner = mock(async () => { throw new Error('prompt mode should not be used'); }); + acpSessionUpdates = [ + { + update: { + sessionUpdate: 'tool_call', + toolCallId: 'tc-1', + status: 'completed', + title: 'Read', + rawInput: { path: '.agents/skills/csv-analyzer/SKILL.md' }, + rawOutput: 'skill content', + }, + }, + ...acpSessionUpdates, + ]; const provider = new CopilotCliProvider( 'copilot-cli-custom', { @@ -209,6 +222,17 @@ describe('CopilotCliProvider custom provider ACP mode', () => { }); expect(extractLastAssistantContent(response.output)).toBe('agentv-copilot-gateway-ok'); + expect(response.metadata?.skillCalls).toEqual([ + { + name: 'csv-analyzer', + input: { + path: '.agents/skills/csv-analyzer/SKILL.md', + file_path: '.agents/skills/csv-analyzer/SKILL.md', + }, + path: '.agents/skills/csv-analyzer/SKILL.md', + source: 'heuristic', + }, + ]); expect(response.targetExecution?.status).toBe('success'); expect(response.targetExecution?.providerKind).toBe('copilot-cli'); expect(runner).not.toHaveBeenCalled(); diff --git a/packages/core/test/evaluation/providers/copilot-sdk.test.ts b/packages/core/test/evaluation/providers/copilot-sdk.test.ts index a784486fd..cb347b611 100644 --- a/packages/core/test/evaluation/providers/copilot-sdk.test.ts +++ b/packages/core/test/evaluation/providers/copilot-sdk.test.ts @@ -455,7 +455,11 @@ describe('CopilotSdkProvider', () => { events: [ { type: 'tool.execution_start', - data: { toolCallId: 'tc-1', toolName: 'Read', input: { path: '/foo.ts' } }, + data: { + toolCallId: 'tc-1', + toolName: 'Read', + input: { path: '.agents/skills/csv-analyzer/SKILL.md' }, + }, }, { type: 'tool.execution_end', @@ -480,10 +484,24 @@ describe('CopilotSdkProvider', () => { expect(msg?.toolCalls).toBeDefined(); expect(msg?.toolCalls?.length).toBe(1); expect(msg?.toolCalls?.[0]?.tool).toBe('Read'); - expect(msg?.toolCalls?.[0]?.input).toEqual({ path: '/foo.ts', file_path: '/foo.ts' }); + expect(msg?.toolCalls?.[0]?.input).toEqual({ + path: '.agents/skills/csv-analyzer/SKILL.md', + file_path: '.agents/skills/csv-analyzer/SKILL.md', + }); expect(msg?.toolCalls?.[0]?.output).toBe('file content'); expect(msg?.toolCalls?.[0]?.id).toBe('tc-1'); expect(msg?.toolCalls?.[0]?.durationMs).toBeDefined(); + expect(response.metadata?.skillCalls).toEqual([ + { + name: 'csv-analyzer', + input: { + path: '.agents/skills/csv-analyzer/SKILL.md', + file_path: '.agents/skills/csv-analyzer/SKILL.md', + }, + path: '.agents/skills/csv-analyzer/SKILL.md', + source: 'heuristic', + }, + ]); }); it('auto-approves permission requests', async () => { diff --git a/packages/core/test/evaluation/providers/pi-runtime.test.ts b/packages/core/test/evaluation/providers/pi-runtime.test.ts index dd292510e..b230344a9 100644 --- a/packages/core/test/evaluation/providers/pi-runtime.test.ts +++ b/packages/core/test/evaluation/providers/pi-runtime.test.ts @@ -55,6 +55,45 @@ describe('Pi coding-agent runtime providers', () => { } }); + it('emits skillCalls metadata from pi-cli tool events', async () => { + const workspace = await mkdtemp(path.join(tmpdir(), 'agentv-pi-cli-test-')); + const provider = new PiCliProvider('pi-cli-target', baseCliConfig(), async () => ({ + stdout: `${JSON.stringify({ + type: 'tool_execution_start', + toolName: 'read', + toolCallId: 'tc-1', + args: { path: '.agents/skills/csv-analyzer/SKILL.md' }, + })}\n${JSON.stringify({ + type: 'tool_execution_end', + toolCallId: 'tc-1', + result: 'skill content', + })}\n${JSON.stringify({ + type: 'agent_end', + messages: [{ role: 'assistant', content: [{ type: 'text', text: 'done' }] }], + })}\n`, + stderr: '', + exitCode: 0, + })); + + try { + const response = await provider.invoke({ question: 'hello', cwd: workspace }); + + expect(response.metadata?.skillCalls).toEqual([ + { + name: 'csv-analyzer', + input: { + path: '.agents/skills/csv-analyzer/SKILL.md', + file_path: '.agents/skills/csv-analyzer/SKILL.md', + }, + path: '.agents/skills/csv-analyzer/SKILL.md', + source: 'heuristic', + }, + ]); + } finally { + await rm(workspace, { recursive: true, force: true }); + } + }); + it('returns structured pi-cli malformed-output errors', async () => { const workspace = await mkdtemp(path.join(tmpdir(), 'agentv-pi-cli-test-')); const provider = new PiCliProvider('pi-cli-target', baseCliConfig(), async () => ({ diff --git a/packages/core/test/evaluation/providers/sdk-child-provider.test.ts b/packages/core/test/evaluation/providers/sdk-child-provider.test.ts index 23d69028e..399e2989c 100644 --- a/packages/core/test/evaluation/providers/sdk-child-provider.test.ts +++ b/packages/core/test/evaluation/providers/sdk-child-provider.test.ts @@ -68,6 +68,29 @@ describe('SdkChildProvider', () => { }); }); + it('derives skillCalls when child output has tool calls but no metadata', async () => { + const provider = fakeProvider('derive-skill-metadata'); + + const response = await provider.invoke({ question: 'use skill' }); + + expect(response.metadata?.skillCalls).toEqual([ + { + name: 'csv-analyzer', + input: { path: '.agents/skills/csv-analyzer/SKILL.md' }, + path: '.agents/skills/csv-analyzer/SKILL.md', + source: 'heuristic', + }, + ]); + }); + + it('keeps explicit child metadata ahead of derived metadata', async () => { + const provider = fakeProvider('success'); + + const response = await provider.invoke({ question: 'hello' }); + + expect(response.metadata?.skillCalls?.[0]?.name).toBe('codex/list_mcp_resources'); + }); + it('keeps missing SDK dependency errors scoped to the child runner', async () => { const provider = fakeProvider('dependency-error'); @@ -191,6 +214,27 @@ if (mode === 'request-plumbing') { process.exit(0); } +if (mode === 'derive-skill-metadata') { + write({ + type: 'result', + response: { + output: [ + { + role: 'assistant', + content: 'child ok', + toolCalls: [ + { + tool: 'Read', + input: { path: '.agents/skills/csv-analyzer/SKILL.md' }, + }, + ], + }, + ], + }, + }); + process.exit(0); +} + if (mode === 'dependency-error') { write({ type: 'error', diff --git a/packages/core/test/evaluation/providers/skill-calls.test.ts b/packages/core/test/evaluation/providers/skill-calls.test.ts index c1cc87573..40759f004 100644 --- a/packages/core/test/evaluation/providers/skill-calls.test.ts +++ b/packages/core/test/evaluation/providers/skill-calls.test.ts @@ -1,6 +1,9 @@ import { describe, expect, it } from 'bun:test'; -import { deriveSkillCallsFromToolCalls } from '../../../src/evaluation/providers/skill-calls.js'; +import { + deriveSkillCallsFromToolCalls, + skillCallMetadata, +} from '../../../src/evaluation/providers/skill-calls.js'; describe('deriveSkillCallsFromToolCalls', () => { it('preserves explicit provider skill names without path-name validation', () => { @@ -16,4 +19,43 @@ describe('deriveSkillCallsFromToolCalls', () => { }, ]); }); + + it('keeps failed skill reads out of confirmed skillCalls', () => { + const metadata = skillCallMetadata([ + { + name: 'csv-analyzer', + path: '.agents/skills/csv-analyzer/SKILL.md', + source: 'heuristic', + }, + { + name: 'broken-skill', + path: '.agents/skills/broken-skill/SKILL.md', + source: 'heuristic', + isError: true, + }, + ]); + + expect(metadata).toEqual({ + skillCalls: [ + { + name: 'csv-analyzer', + path: '.agents/skills/csv-analyzer/SKILL.md', + source: 'heuristic', + }, + ], + attemptedSkillCalls: [ + { + name: 'csv-analyzer', + path: '.agents/skills/csv-analyzer/SKILL.md', + source: 'heuristic', + }, + { + name: 'broken-skill', + path: '.agents/skills/broken-skill/SKILL.md', + source: 'heuristic', + isError: true, + }, + ], + }); + }); }); diff --git a/packages/sdk/src/assertion.ts b/packages/sdk/src/assertion.ts index 94145fcbd..4c74c3e9c 100644 --- a/packages/sdk/src/assertion.ts +++ b/packages/sdk/src/assertion.ts @@ -55,8 +55,6 @@ export type AssertionType = | 'cost' | 'token-usage' | 'execution-metrics' - /** @deprecated Authored eval YAML rejects this compatibility-only runtime type. */ - | 'skill-trigger' | 'contains' | 'contains-any' | 'contains-all'