diff --git a/AGENTS.md b/AGENTS.md index b835e1064..cc9446195 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -15,13 +15,16 @@ This is a TypeScript monorepo for AgentV - an AI agent evaluation framework. - `packages/core/` - Evaluation engine, providers, grading - `apps/cli/` - Command-line interface (published as `agentv`) -## Essential Commands -- `pnpm install` - Install dependencies -- `pnpm build` - Build all packages -- `pnpm test` - Run tests -- `pnpm typecheck` - Type checking -- `pnpm lint` - Lint code -- `pnpm format` - Format with Prettier +## Quality Assurance Workflow + +After making any significant changes (refactoring, new features, bug fixes), always run the following verification steps in order: + +1. `pnpm build` - Ensure code compiles without errors +2. `pnpm typecheck` - Verify TypeScript type safety across the monorepo +3. `pnpm lint` - Check code style and catch potential issues +4. `pnpm test` - Run all tests to verify functionality + +Only consider the work complete when all four steps pass successfully. This ensures code quality, prevents regressions, and maintains the integrity of the codebase. ## Functional Testing diff --git a/apps/cli/package.json b/apps/cli/package.json index f255ed85d..83ea7b61b 100644 --- a/apps/cli/package.json +++ b/apps/cli/package.json @@ -1,6 +1,6 @@ { "name": "agentv", - "version": "0.15.1", + "version": "0.16.0", "description": "CLI entry point for AgentV", "type": "module", "repository": { diff --git a/apps/cli/src/commands/eval/run-eval.ts b/apps/cli/src/commands/eval/run-eval.ts index f84843656..a218dc823 100644 --- a/apps/cli/src/commands/eval/run-eval.ts +++ b/apps/cli/src/commands/eval/run-eval.ts @@ -2,6 +2,7 @@ import { runEvaluation as defaultRunEvaluation, type EvaluationCache, type EvaluationResult, + type EvalCase, type ProviderResponse, ensureVSCodeSubagents, loadEvalCases, @@ -205,6 +206,7 @@ async function prepareFileMetadata(params: { readonly options: NormalizedOptions; }): Promise<{ readonly evalIds: readonly string[]; + readonly evalCases: readonly EvalCase[]; readonly selection: TargetSelection; readonly inlineTargetLabel: string; }> { @@ -240,7 +242,7 @@ async function prepareFileMetadata(params: { ? evalCases.filter((value) => value.id === options.evalId).map((value) => value.id) : evalCases.map((value) => value.id); - return { evalIds: filteredIds, selection, inlineTargetLabel }; + return { evalIds: filteredIds, evalCases, selection, inlineTargetLabel }; } async function runWithLimit( @@ -276,6 +278,7 @@ async function runSingleEvalFile(params: { readonly displayIdTracker: { getOrAssign(evalKey: string): number }; readonly selection: TargetSelection; readonly inlineTargetLabel: string; + readonly evalCases: readonly EvalCase[]; }): Promise<{ results: EvaluationResult[]; promptDumpDir?: string }> { const { testFilePath, @@ -291,6 +294,7 @@ async function runSingleEvalFile(params: { displayIdTracker, selection, inlineTargetLabel, + evalCases, } = params; @@ -363,6 +367,7 @@ async function runSingleEvalFile(params: { cache, useCache: options.cache, evalId: options.evalId, + evalCases, verbose: options.verbose, maxConcurrency: resolvedWorkers, onResult: async (result: EvaluationResult) => { @@ -420,7 +425,7 @@ export async function runEvalCommand(input: RunEvalCommandInput): Promise : undefined; const fileMetadata = new Map< string, - { readonly evalIds: readonly string[]; readonly selection: TargetSelection; readonly inlineTargetLabel: string } + { readonly evalIds: readonly string[]; readonly evalCases: readonly EvalCase[]; readonly selection: TargetSelection; readonly inlineTargetLabel: string } >(); for (const testFilePath of resolvedTestFiles) { const meta = await prepareFileMetadata({ @@ -483,6 +488,7 @@ export async function runEvalCommand(input: RunEvalCommandInput): Promise displayIdTracker, selection: targetPrep.selection, inlineTargetLabel: targetPrep.inlineTargetLabel, + evalCases: targetPrep.evalCases, }); allResults.push(...result.results); diff --git a/apps/cli/test/fixtures/mock-run-evaluation.ts b/apps/cli/test/fixtures/mock-run-evaluation.ts index 08f54560d..26a4139e6 100644 --- a/apps/cli/test/fixtures/mock-run-evaluation.ts +++ b/apps/cli/test/fixtures/mock-run-evaluation.ts @@ -15,6 +15,8 @@ interface RunEvaluationOptionsLike { readonly cache?: unknown; readonly useCache?: boolean; readonly testId?: string; + readonly evalId?: string; + readonly evalCases?: ReadonlyArray; readonly verbose?: boolean; readonly onResult?: (result: EvaluationResultLike) => Promise | void; } diff --git a/openspec/changes/add-custom-evaluator-validation/proposal.md b/openspec/changes/add-custom-evaluator-validation/proposal.md new file mode 100644 index 000000000..e9f4028e7 --- /dev/null +++ b/openspec/changes/add-custom-evaluator-validation/proposal.md @@ -0,0 +1,48 @@ +# Proposal: Add Custom Evaluator Template Validation + +## Summary + +Add validation warnings when custom LLM judge evaluator templates are missing required template variables, helping users create effective evaluator prompts that have something to evaluate against. + +## Problem + +Custom evaluator templates can be created without including the essential fields needed to perform evaluation. When a template contains only variables like `{{ question }}` without `{{ candidate_answer }}` or `{{ expected_messages }}`, the LLM judge receives no content to actually evaluate, resulting in meaningless or inconsistent evaluations. + +## Solution + +Add validation logic in `resolveCustomPrompt()` that checks custom evaluator templates (both file-based and inline) for the presence of at least one required evaluation field: +- `{{ candidate_answer }}` - to evaluate the agent's response +- `{{ expected_messages }}` - to compare against expected output + +When neither field is present, display a clear warning message explaining what's missing and why it's needed. + +## Impact + +### User Impact +- **Improved UX**: Users get immediate feedback when their evaluator templates are incomplete +- **Better Documentation**: Clear guidance on what fields are required +- **Prevents Silent Failures**: Catches incomplete templates before they produce poor evaluations + +### System Impact +- **Minimal**: Validation runs during template loading, adds negligible overhead +- **Non-breaking**: Warning only, doesn't block evaluation execution + +## Scope + +### In Scope +- Validation function to check for required template variables +- Warning messages for file-based and inline prompts +- Unit tests covering validation scenarios +- Documentation updates in custom-evaluators.md + +### Out of Scope +- Validation of other template variables +- Enforcement of evaluation criteria or scoring rubrics +- Validation of code-based evaluators + +## Implementation Notes + +- Validation occurs in `packages/core/src/evaluation/orchestrator.ts` +- Uses regex pattern matching to detect template variables +- Warnings use ANSI color codes (yellow) for visibility +- Permissive approach: warns but allows evaluation to continue diff --git a/openspec/changes/add-custom-evaluator-validation/specs/validation/spec.md b/openspec/changes/add-custom-evaluator-validation/specs/validation/spec.md new file mode 100644 index 000000000..b04473e81 --- /dev/null +++ b/openspec/changes/add-custom-evaluator-validation/specs/validation/spec.md @@ -0,0 +1,64 @@ +# Validation Capability Delta + +## Why + +Custom LLM judge evaluator templates can be created without the essential fields needed to perform evaluation. When a template contains only `{{ question }}` without `{{ candidate_answer }}` or `{{ expected_messages }}`, the LLM judge has no content to evaluate against, resulting in meaningless evaluations. + +This validation provides immediate feedback to help users create effective evaluator templates. + +## ADDED Requirements + +### Requirement: Custom Evaluator Template Validation + +The system SHALL validate custom LLM judge evaluator templates to ensure they contain fields necessary for evaluation. + +#### Scenario: Template missing both required fields shows warning + +**Given** a custom evaluator template with content `"{{ question }}"` +**When** the template is loaded +**Then** a warning is displayed containing: +- Message: "Custom evaluator template at [source] is missing required fields" +- List of required fields: `{{ candidate_answer }}` and `{{ expected_messages }}` +- Explanation: "Without these, there is nothing to evaluate against" + +#### Scenario: Template with candidate_answer does not warn + +**Given** a custom evaluator template containing `"{{ candidate_answer }}"` +**When** the template is loaded +**Then** no validation warning is displayed + +#### Scenario: Template with expected_messages does not warn + +**Given** a custom evaluator template containing `"{{ expected_messages }}"` +**When** the template is loaded +**Then** no validation warning is displayed + +#### Scenario: Validation applies to file-based prompts + +**Given** an evaluator configured with `promptPath: "./my-eval.md"` +**And** the file contains only `"{{ question }}"` +**When** the custom prompt is resolved +**Then** a warning is displayed referencing the file path + +#### Scenario: Invalid template variables are detected + +**Given** a custom evaluator template containing `"{{ candiate_answer }} for {{ invalid_var }}"` +**When** validation runs +**Then** a warning is displayed listing the invalid variables +**And** the warning lists all valid template variables + +#### Scenario: Validation is permissive + +**Given** a custom evaluator template missing required fields +**When** validation runs +**Then** a warning is displayed +**But** evaluation continues without blocking + +## Implementation Notes + +- Validation occurs in `validateCustomPromptContent()` function +- Uses regex patterns to detect template variables: `/\{\{\s*(candidate_answer|expected_messages)\s*\}\}/` +- Validates all template variables against a whitelist of valid names +- Valid variables: `candidate_answer`, `expected_messages`, `question`, `expected_outcome`, `reference_answer`, `input_messages`, `output_messages` (legacy) +- Warnings use ANSI yellow color codes for visibility +- File-based (`promptPath`) evaluators are validated diff --git a/openspec/changes/add-custom-evaluator-validation/tasks.md b/openspec/changes/add-custom-evaluator-validation/tasks.md new file mode 100644 index 000000000..78af97b37 --- /dev/null +++ b/openspec/changes/add-custom-evaluator-validation/tasks.md @@ -0,0 +1,28 @@ +# Tasks: Add Custom Evaluator Template Validation + +## Implementation Tasks + +- [x] Add `validateCustomPromptContent()` function to check for required template variables +- [x] Add validation for invalid/unknown template variables (catches typos) +- [x] Update `resolveCustomPrompt()` to call validation for both file-based and inline prompts +- [x] Add ANSI color constants for warning output +- [x] Create warning message explaining required fields +- [x] Create warning message for invalid variables +- [x] Add unit test: warns when template is missing required fields +- [x] Add unit test: no warning when `{{ candidate_answer }}` is present +- [x] Add unit test: no warning when `{{ expected_messages }}` is present +- [x] Add unit test: warns when template contains invalid variables (typos, undefined vars) +- [x] Add unit test: validates inline prompts in addition to file-based prompts +- [x] Fix lint errors (ESLint no-explicit-any warnings) +- [x] Fix TypeScript compilation errors +- [x] Verify all tests pass +- [x] Update documentation to reflect validation behavior + +## Validation + +- [x] All unit tests pass (13 tests in orchestrator.test.ts) +- [x] Lint passes (`pnpm lint`) +- [x] TypeScript compilation passes (`pnpm typecheck`) +- [x] Warning appears for incomplete templates +- [x] Warning appears for invalid/unknown template variables +- [x] No warnings for valid templates with required fields diff --git a/openspec/changes/rename-output-messages-variable/proposal.md b/openspec/changes/rename-output-messages-variable/proposal.md new file mode 100644 index 000000000..24918dcd5 --- /dev/null +++ b/openspec/changes/rename-output-messages-variable/proposal.md @@ -0,0 +1,58 @@ +# Proposal: Rename output_messages Template Variable to expected_messages + +## Summary + +Rename the LLM judge template variable from `{{ output_messages }}` to `{{ expected_messages }}` for consistency with eval file schema and clearer semantics. + +## Problem + +The template variable name `output_messages` is inconsistent and confusing: +- **Eval schema uses**: `expected_messages` field (not `output_messages`) +- **Validation checks for**: `{{ expected_messages }}` template variable +- **Documentation shows**: `{{output_messages}}` as available variable +- **Name is ambiguous**: "output" could refer to agent output or expected output + +This inconsistency creates confusion when writing custom evaluator templates. + +## Solution + +Rename `output_messages` to `expected_messages` throughout the codebase: +1. Template variable substitution in evaluators +2. Documentation references +3. Keep backward compatibility during transition (optional - TBD) + +## Impact + +### User Impact +- **Breaking Change**: Existing custom evaluator templates using `{{ output_messages }}` will need updates +- **Migration Path**: Users must update their templates to use `{{ expected_messages }}` +- **Documentation**: All examples and guides updated to reflect new name + +### System Impact +- **Code Changes**: Update variable name in evaluator template substitution +- **Documentation**: Update custom-evaluators.md and related docs +- **Tests**: Update test fixtures and expectations + +## Scope + +### In Scope +- Rename template variable in `evaluators.ts` +- Update documentation in `.claude/skills/agentv-eval-builder/references/custom-evaluators.md` +- Update any example evaluator templates +- Validation already checks for `expected_messages` (no change needed) + +### Out of Scope +- Backward compatibility support (decide if needed) +- Changes to eval file schema (already uses `expected_messages`) +- Changes to validation logic (already correct) + +## Questions for Decision + +1. **Backward Compatibility**: Should we support both `{{ output_messages }}` and `{{ expected_messages }}` temporarily? + - **Option A**: Breaking change - only support new name + - **Option B**: Support both with deprecation warning + - **Recommendation**: Option A (breaking change) - simpler, cleaner + +2. **Migration Timing**: When to release this change? + - Should coordinate with next major/minor version + - Include in release notes with migration guide diff --git a/openspec/changes/rename-output-messages-variable/specs/evaluation/spec.md b/openspec/changes/rename-output-messages-variable/specs/evaluation/spec.md new file mode 100644 index 000000000..442f0d8c0 --- /dev/null +++ b/openspec/changes/rename-output-messages-variable/specs/evaluation/spec.md @@ -0,0 +1,42 @@ +# Evaluation Capability Delta + +## Why + +The template variable name `output_messages` is inconsistent with the eval file schema field `expected_messages` and creates confusion. Renaming to `expected_messages` provides consistency across the system and clearer semantics (these are the expected messages, not just output). + +## MODIFIED Requirements + +### Requirement: LLM Judge Template Variables + +The system SHALL provide template variables for LLM judge custom evaluator prompts to reference evaluation context. + +**Change**: Renamed `{{ output_messages }}` variable to `{{ expected_messages }}` for consistency with eval schema. + +#### Scenario: expected_messages variable available + +**Given** a custom LLM judge evaluator template +**When** template substitution occurs +**Then** the `{{ expected_messages }}` variable contains JSON stringified output segments + +**Note**: Previously named `{{ output_messages }}` - renamed for consistency with eval schema + +#### Scenario: Template variable substitution + +**Given** an eval case with `output_segments` data +**And** a custom evaluator template containing `{{ expected_messages }}` +**When** the template is rendered +**Then** `{{ expected_messages }}` is replaced with `JSON.stringify(output_segments, null, 2)` + +#### Scenario: Backward compatibility not supported + +**Given** a custom evaluator template containing `{{ output_messages }}` +**When** template substitution occurs +**Then** the variable is NOT substituted (remains as literal text) +**And** validation may warn about missing required fields + +## Implementation Notes + +- Update variable name in `evaluators.ts` template substitution logic +- Update documentation to reflect new variable name +- This is a breaking change - existing templates must update to use `{{ expected_messages }}` +- Validation already checks for `{{ expected_messages }}` (no changes needed) diff --git a/openspec/changes/rename-output-messages-variable/tasks.md b/openspec/changes/rename-output-messages-variable/tasks.md new file mode 100644 index 000000000..512faf427 --- /dev/null +++ b/openspec/changes/rename-output-messages-variable/tasks.md @@ -0,0 +1,31 @@ +# Tasks: Rename output_messages Template Variable + +## Implementation Tasks + +- [ ] Update variable name in `packages/core/src/evaluation/evaluators.ts` (line ~105) + - Change `output_messages` to `expected_messages` in template variable substitution +- [ ] Update documentation in `.claude/skills/agentv-eval-builder/references/custom-evaluators.md` + - Change `{{output_messages}}` to `{{expected_messages}}` in template variables list +- [ ] Search for any other references to `output_messages` in documentation + - Check README files + - Check example evaluator templates + - Check inline code comments +- [ ] Update tests if they reference the old variable name + - Check test fixtures + - Check test assertions +- [ ] Verify validation still works correctly (already checks for `expected_messages`) + +## Validation + +- [ ] All unit tests pass +- [ ] Lint passes (`pnpm lint`) +- [ ] TypeScript compilation passes (`pnpm typecheck`) +- [ ] Custom evaluator templates work with `{{ expected_messages }}` +- [ ] Documentation accurately reflects the change +- [ ] No references to `output_messages` remain in user-facing content + +## Migration Notes + +- **Breaking Change**: Existing custom evaluator templates using `{{ output_messages }}` will need to be updated +- Include in release notes with clear migration instructions +- Consider version bump (minor or major depending on project versioning policy) diff --git a/packages/core/package.json b/packages/core/package.json index 988143dbd..5a1046ae5 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -1,6 +1,6 @@ { "name": "@agentv/core", - "version": "0.15.1", + "version": "0.16.0", "description": "Primitive runtime components for AgentV", "type": "module", "repository": { diff --git a/packages/core/src/evaluation/evaluators.ts b/packages/core/src/evaluation/evaluators.ts index c5f4d00b4..78b46da56 100644 --- a/packages/core/src/evaluation/evaluators.ts +++ b/packages/core/src/evaluation/evaluators.ts @@ -1,5 +1,6 @@ import type { ResolvedTarget } from "./providers/targets.js"; import type { Provider, ProviderResponse, ChatPrompt } from "./providers/types.js"; +import { TEMPLATE_VARIABLES } from "./template-variables.js"; import type { EvaluatorConfig, JsonObject, EvalCase } from "./types.js"; /** @@ -13,16 +14,16 @@ Use the reference_answer as a gold standard for a high-quality response (if prov Be concise and focused in your evaluation. Provide succinct, specific feedback rather than verbose explanations. [[ ## expected_outcome ## ]] -{{expected_outcome}} +{{${TEMPLATE_VARIABLES.EXPECTED_OUTCOME}}} [[ ## question ## ]] -{{question}} +{{${TEMPLATE_VARIABLES.QUESTION}}} [[ ## reference_answer ## ]] -{{reference_answer}} +{{${TEMPLATE_VARIABLES.REFERENCE_ANSWER}}} [[ ## candidate_answer ## ]] -{{candidate_answer}}`; +{{${TEMPLATE_VARIABLES.CANDIDATE_ANSWER}}}`; export interface EvaluationContext { readonly evalCase: EvalCase; @@ -101,12 +102,12 @@ export class LlmJudgeEvaluator implements Evaluator { // Prepare template variables for substitution const variables = { - input_messages: JSON.stringify(context.evalCase.input_segments, null, 2), - output_messages: JSON.stringify(context.evalCase.output_segments, null, 2), - candidate_answer: context.candidate.trim(), - reference_answer: (context.evalCase.reference_answer ?? "").trim(), - expected_outcome: context.evalCase.expected_outcome.trim(), - question: formattedQuestion.trim(), + [TEMPLATE_VARIABLES.INPUT_MESSAGES]: JSON.stringify(context.evalCase.input_segments, null, 2), + [TEMPLATE_VARIABLES.EXPECTED_MESSAGES]: JSON.stringify(context.evalCase.expected_segments, null, 2), + [TEMPLATE_VARIABLES.CANDIDATE_ANSWER]: context.candidate.trim(), + [TEMPLATE_VARIABLES.REFERENCE_ANSWER]: (context.evalCase.reference_answer ?? "").trim(), + [TEMPLATE_VARIABLES.EXPECTED_OUTCOME]: context.evalCase.expected_outcome.trim(), + [TEMPLATE_VARIABLES.QUESTION]: formattedQuestion.trim(), }; // Build system prompt (only the mandatory output schema) @@ -424,7 +425,7 @@ function parseJsonSafe(payload: string): Record | undefined { } function substituteVariables(template: string, variables: Record): string { - return template.replace(/\{\{([a-zA-Z0-9_]+)\}\}/g, (match, varName) => { + return template.replace(/\{\{\s*([a-zA-Z0-9_]+)\s*\}\}/g, (match, varName) => { return variables[varName] ?? match; }); } diff --git a/packages/core/src/evaluation/loaders/evaluator-parser.ts b/packages/core/src/evaluation/loaders/evaluator-parser.ts index 3f7618a66..2dab08b4f 100644 --- a/packages/core/src/evaluation/loaders/evaluator-parser.ts +++ b/packages/core/src/evaluation/loaders/evaluator-parser.ts @@ -1,8 +1,9 @@ import path from "node:path"; -import { resolveFileReference } from "./file-resolver.js"; import type { EvaluatorConfig, EvaluatorKind, JsonObject, JsonValue } from "../types.js"; import { isEvaluatorKind } from "../types.js"; +import { resolveFileReference } from "./file-resolver.js"; +import { validateCustomPromptContent } from "../validation/prompt-validator.js"; const ANSI_YELLOW = "\u001b[33m"; const ANSI_RESET = "\u001b[0m"; @@ -91,6 +92,14 @@ export async function parseEvaluators( const resolved = await resolveFileReference(prompt, searchRoots); if (resolved.resolvedPath) { promptPath = path.resolve(resolved.resolvedPath); + // Validate custom prompt content upfront - throws error if validation fails + try { + await validateCustomPromptContent(promptPath); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + // Add context and re-throw for the caller to handle + throw new Error(`Evaluator '${name}' template (${promptPath}): ${message}`); + } } else { logWarning( `Inline prompt used for evaluator '${name}' in '${evalId}' (file not found: ${resolved.displayPath})`, diff --git a/packages/core/src/evaluation/orchestrator.ts b/packages/core/src/evaluation/orchestrator.ts index b57885ad8..a2eba5fea 100644 --- a/packages/core/src/evaluation/orchestrator.ts +++ b/packages/core/src/evaluation/orchestrator.ts @@ -66,6 +66,7 @@ export interface RunEvaluationOptions { readonly evalId?: string; readonly verbose?: boolean; readonly maxConcurrency?: number; + readonly evalCases?: readonly EvalCase[]; readonly onResult?: (result: EvaluationResult) => MaybePromise; readonly onProgress?: (event: ProgressEvent) => MaybePromise; } @@ -87,12 +88,13 @@ export async function runEvaluation(options: RunEvaluationOptions): Promise { if (config.promptPath) { try { - return await readTextFile(config.promptPath); + const content = await readTextFile(config.promptPath); + return content; } catch (error) { const message = error instanceof Error ? error.message : String(error); console.warn(`Could not read custom prompt at ${config.promptPath}: ${message}`); @@ -818,6 +821,8 @@ async function resolveCustomPrompt(config: { readonly prompt?: string; readonly return config.prompt; } + + function isNonEmptyString(value: unknown): value is string { return typeof value === "string" && value.trim().length > 0; } diff --git a/packages/core/src/evaluation/template-variables.ts b/packages/core/src/evaluation/template-variables.ts new file mode 100644 index 000000000..a5d9ff550 --- /dev/null +++ b/packages/core/src/evaluation/template-variables.ts @@ -0,0 +1,33 @@ +/** + * Template variable constants for evaluator prompts. + * These variables can be used in custom evaluator templates with {{ variable_name }} syntax. + */ +export const TEMPLATE_VARIABLES = { + CANDIDATE_ANSWER: 'candidate_answer', + EXPECTED_MESSAGES: 'expected_messages', + QUESTION: 'question', + EXPECTED_OUTCOME: 'expected_outcome', + REFERENCE_ANSWER: 'reference_answer', + INPUT_MESSAGES: 'input_messages', +} as const; + +/** + * Type representing all valid template variable names. + */ +export type TemplateVariable = typeof TEMPLATE_VARIABLES[keyof typeof TEMPLATE_VARIABLES]; + +/** + * Set of all valid template variable names for runtime validation. + */ +export const VALID_TEMPLATE_VARIABLES = new Set( + Object.values(TEMPLATE_VARIABLES) +); + +/** + * Template variables that are required for meaningful evaluation. + * At least one of these should be present in a custom evaluator template. + */ +export const REQUIRED_TEMPLATE_VARIABLES = new Set([ + TEMPLATE_VARIABLES.CANDIDATE_ANSWER, + TEMPLATE_VARIABLES.EXPECTED_MESSAGES, +]); diff --git a/packages/core/src/evaluation/types.ts b/packages/core/src/evaluation/types.ts index 3b0e1d83c..263339601 100644 --- a/packages/core/src/evaluation/types.ts +++ b/packages/core/src/evaluation/types.ts @@ -173,7 +173,7 @@ export interface EvalCase { readonly question: string; readonly input_messages: readonly TestMessage[]; readonly input_segments: readonly JsonObject[]; - readonly output_segments: readonly JsonObject[]; + readonly expected_segments: readonly JsonObject[]; readonly reference_answer?: string; readonly guideline_paths: readonly string[]; readonly guideline_patterns?: readonly string[]; diff --git a/packages/core/src/evaluation/validation/prompt-validator.ts b/packages/core/src/evaluation/validation/prompt-validator.ts new file mode 100644 index 000000000..c2e1af3f6 --- /dev/null +++ b/packages/core/src/evaluation/validation/prompt-validator.ts @@ -0,0 +1,60 @@ +import { readFile } from "node:fs/promises"; + +import { VALID_TEMPLATE_VARIABLES, TEMPLATE_VARIABLES } from "../template-variables.js"; + +const ANSI_YELLOW = "\u001b[33m"; +const ANSI_RESET = "\u001b[0m"; + +/** + * Validate custom prompt template content from a file. + * Reads the file and checks for valid template variables. + * Throws an error if required template variables are missing. + */ +export async function validateCustomPromptContent(promptPath: string): Promise { + const content = await readFile(promptPath, "utf8"); + validateTemplateVariables(content, promptPath); +} + +/** + * Validate template variables in a custom prompt template string. + * Exported for testing purposes. + * @throws Error if required template variables are missing + */ +export function validateTemplateVariables(content: string, source: string): void { + // Extract all template variables from content + const variablePattern = /\{\{\s*([a-zA-Z0-9_]+)\s*\}\}/g; + const foundVariables = new Set(); + const invalidVariables: string[] = []; + + let match; + while ((match = variablePattern.exec(content)) !== null) { + const varName = match[1]; + foundVariables.add(varName); + if (!VALID_TEMPLATE_VARIABLES.has(varName)) { + invalidVariables.push(varName); + } + } + + // Check if template contains required variables for evaluation + const hasCandidateAnswer = foundVariables.has(TEMPLATE_VARIABLES.CANDIDATE_ANSWER); + const hasExpectedMessages = foundVariables.has(TEMPLATE_VARIABLES.EXPECTED_MESSAGES); + const hasRequiredFields = hasCandidateAnswer || hasExpectedMessages; + + // ERROR: Missing required fields - throw error to skip this evaluator/eval case + if (!hasRequiredFields) { + throw new Error( + `Missing required fields. Must include at least one of:\n` + + ` - {{ ${TEMPLATE_VARIABLES.CANDIDATE_ANSWER} }}\n` + + ` - {{ ${TEMPLATE_VARIABLES.EXPECTED_MESSAGES} }}` + ); + } + + // WARNING: Invalid variables - show warning but continue + if (invalidVariables.length > 0) { + const warningMessage = `${ANSI_YELLOW}Warning: Custom evaluator template at ${source} + Contains invalid variables: ${invalidVariables.map(v => `{{ ${v} }}`).join(', ')} + Valid variables: ${Array.from(VALID_TEMPLATE_VARIABLES).map(v => `{{ ${v} }}`).join(', ')}${ANSI_RESET}`; + + console.warn(warningMessage); + } +} diff --git a/packages/core/src/evaluation/yaml-parser.ts b/packages/core/src/evaluation/yaml-parser.ts index 129116fa2..eee2e1814 100644 --- a/packages/core/src/evaluation/yaml-parser.ts +++ b/packages/core/src/evaluation/yaml-parser.ts @@ -21,6 +21,7 @@ export { extractCodeBlocks } from "./formatting/segment-formatter.js"; export { isGuidelineFile } from "./loaders/config-loader.js"; const ANSI_YELLOW = "\u001b[33m"; +const ANSI_RED = "\u001b[31m"; const ANSI_RESET = "\u001b[0m"; const SCHEMA_EVAL_V2 = "agentv-eval-v2"; @@ -143,7 +144,7 @@ export async function loadEvalCases( const expectedMessagesValue = evalcase.expected_messages; if (!id || !outcome || !Array.isArray(inputMessagesValue)) { - logWarning(`Skipping incomplete eval case: ${id ?? "unknown"}`); + logError(`Skipping incomplete eval case: ${id ?? "unknown"}. Missing required fields: id, outcome, and/or input_messages`); continue; } @@ -157,7 +158,7 @@ export async function loadEvalCases( : []; if (hasExpectedMessages && expectedMessages.length === 0) { - logWarning(`No valid expected message found for eval case: ${id}`); + logError(`No valid expected message found for eval case: ${id}`); continue; } @@ -203,7 +204,15 @@ export async function loadEvalCases( .join(" "); const evalCaseEvaluatorKind = coerceEvaluator(evalcase.evaluator, id) ?? globalEvaluator; - const evaluators = await parseEvaluators(evalcase, globalExecution, searchRoots, id ?? "unknown"); + let evaluators: Awaited>; + try { + evaluators = await parseEvaluators(evalcase, globalExecution, searchRoots, id ?? "unknown"); + } catch (error) { + // Skip entire eval case if evaluator validation fails + const message = error instanceof Error ? error.message : String(error); + logError(`Skipping eval case '${id}': ${message}`); + continue; + } // Extract file paths from all input segments (non-guideline files) const userFilePaths: string[] = []; @@ -226,7 +235,7 @@ export async function loadEvalCases( question: question, input_messages: inputMessages, input_segments: inputSegments, - output_segments: outputSegments, + expected_segments: outputSegments, reference_answer: referenceAnswer, guideline_paths: guidelinePaths.map((guidelinePath) => path.resolve(guidelinePath)), guideline_patterns: guidelinePatterns, @@ -267,3 +276,12 @@ function logWarning(message: string, details?: readonly string[]): void { console.warn(`${ANSI_YELLOW}Warning: ${message}${ANSI_RESET}`); } } + +function logError(message: string, details?: readonly string[]): void { + if (details && details.length > 0) { + const detailBlock = details.join("\n"); + console.error(`${ANSI_RED}Error: ${message}\n${detailBlock}${ANSI_RESET}`); + } else { + console.error(`${ANSI_RED}Error: ${message}${ANSI_RESET}`); + } +} diff --git a/packages/core/test/evaluation/evaluators.test.ts b/packages/core/test/evaluation/evaluators.test.ts index a012b230b..9e6c4ac03 100644 --- a/packages/core/test/evaluation/evaluators.test.ts +++ b/packages/core/test/evaluation/evaluators.test.ts @@ -37,7 +37,7 @@ const baseTestCase: EvalCase = { question: "Improve the logging implementation", input_messages: [{ role: "user", content: "Please add logging" }], input_segments: [{ type: "text", value: "Please add logging" }], - output_segments: [], + expected_segments: [], reference_answer: "- add structured logging\n- avoid global state", guideline_paths: [], file_paths: [], diff --git a/packages/core/test/evaluation/evaluators_variables.test.ts b/packages/core/test/evaluation/evaluators_variables.test.ts index 975d8030a..1480b34a6 100644 --- a/packages/core/test/evaluation/evaluators_variables.test.ts +++ b/packages/core/test/evaluation/evaluators_variables.test.ts @@ -24,8 +24,8 @@ const baseTestCase: EvalCase = { dataset: "test-dataset", question: "Original Question Text", input_messages: [{ role: "user", content: "User Input Message" }], - input_segments: [{ type: "text", value: "User Input Message" }], - output_segments: [{ type: "text", value: "Expected Output Message" }], + input_segments: [{ type: "text", value: "Input Message" }], + expected_segments: [{ type: "text", value: "Expected Output Message" }], reference_answer: "Reference Answer Text", guideline_paths: [], file_paths: [], @@ -49,7 +49,7 @@ Outcome: {{expected_outcome}} Reference: {{reference_answer}} Candidate: {{candidate_answer}} Input Messages: {{input_messages}} -Output Messages: {{output_messages}} +Expected Messages: {{expected_messages}} `; const judgeProvider = new CapturingProvider({ @@ -91,10 +91,10 @@ Output Messages: {{output_messages}} // Verify input_messages JSON stringification expect(request?.question).toContain('Input Messages: ['); - expect(request?.question).toContain('"value": "User Input Message"'); + expect(request?.question).toContain('"value": "Input Message"'); - // Verify output_messages JSON stringification - expect(request?.question).toContain('Output Messages: ['); + // Verify expected_messages JSON stringification + expect(request?.question).toContain('Expected Messages: ['); expect(request?.question).toContain('"value": "Expected Output Message"'); // System prompt only has output schema, not custom template @@ -134,4 +134,60 @@ Output Messages: {{output_messages}} expect(request?.systemPrompt).toContain("You must respond with a single JSON object"); expect(request?.systemPrompt).not.toContain(customPrompt); }); + + it("substitutes template variables with whitespace inside braces", async () => { + const formattedQuestion = `What is the status?`; + const customPrompt = ` +Question: {{ question }} +Outcome: {{ expected_outcome }} +Reference: {{ reference_answer }} +Candidate: {{ candidate_answer }} +Input Messages: {{ input_messages }} +Expected Messages: {{ expected_messages }} +`; + + const judgeProvider = new CapturingProvider({ + text: JSON.stringify({ + score: 0.8, + hits: ["Good"], + misses: [], + reasoning: "Reasoning", + }), + }); + + const evaluator = new LlmJudgeEvaluator({ + resolveJudgeProvider: async () => judgeProvider, + evaluatorTemplate: customPrompt, + }); + + const candidateAnswer = "Candidate Answer Text"; + + await evaluator.evaluate({ + evalCase: { ...baseTestCase, evaluator: "llm_judge" }, + candidate: candidateAnswer, + target: baseTarget, + provider: judgeProvider, + attempt: 0, + promptInputs: { question: formattedQuestion, guidelines: "" }, + now: new Date(), + }); + + const request = judgeProvider.lastRequest; + expect(request).toBeDefined(); + + // Verify all variables were substituted despite whitespace + expect(request?.question).toContain(`Question: ${formattedQuestion}`); + expect(request?.question).toContain("Outcome: Expected Outcome Text"); + expect(request?.question).toContain("Reference: Reference Answer Text"); + expect(request?.question).toContain("Candidate: Candidate Answer Text"); + + // Verify JSON stringified variables were also substituted + expect(request?.question).toContain('Input Messages: ['); + expect(request?.question).toContain('"value": "Input Message"'); + expect(request?.question).toContain('Expected Messages: ['); + expect(request?.question).toContain('"value": "Expected Output Message"'); + + // Verify no unreplaced template markers remain + expect(request?.question).not.toMatch(/\{\{\s*\w+\s*\}\}/); + }); }); diff --git a/packages/core/test/evaluation/orchestrator.test.ts b/packages/core/test/evaluation/orchestrator.test.ts index d1f1c3b86..26f818393 100644 --- a/packages/core/test/evaluation/orchestrator.test.ts +++ b/packages/core/test/evaluation/orchestrator.test.ts @@ -80,7 +80,7 @@ const baseTestCase: EvalCase = { question: "Explain logging improvements", input_messages: [{ role: "user", content: "Explain logging improvements" }], input_segments: [{ type: "text", value: "Explain logging improvements" }], - output_segments: [], + expected_segments: [], reference_answer: "- add structured logging\n- avoid global state", guideline_paths: [], file_paths: [], @@ -233,7 +233,7 @@ describe("runTestCase", () => { it("uses a custom evaluator prompt when provided", async () => { const directory = mkdtempSync(path.join(tmpdir(), "agentv-custom-judge-")); const promptPath = path.join(directory, "judge.md"); - writeFileSync(promptPath, "CUSTOM PROMPT CONTENT", "utf8"); + writeFileSync(promptPath, "CUSTOM PROMPT CONTENT with {{ candidate_answer }}", "utf8"); const provider = new SequenceProvider("mock", { responses: [{ text: "Answer text" }], @@ -294,7 +294,7 @@ describe("runTestCase", () => { { type: "text", value: "Review" }, { type: "text", value: "Ack" }, ], - output_segments: [], + expected_segments: [], reference_answer: "", guideline_paths: [], file_paths: [], @@ -326,7 +326,7 @@ describe("runTestCase", () => { question: "", input_messages: [{ role: "user", content: "Hello" }], input_segments: [{ type: "text", value: "Hello" }], - output_segments: [], + expected_segments: [], reference_answer: "", guideline_paths: [], file_paths: [], diff --git a/packages/core/test/evaluation/yaml-parser.test.ts b/packages/core/test/evaluation/yaml-parser.test.ts index a40e6e9c8..3e7aee07f 100644 --- a/packages/core/test/evaluation/yaml-parser.test.ts +++ b/packages/core/test/evaluation/yaml-parser.test.ts @@ -105,7 +105,7 @@ describe("buildPromptInputs chatPrompt", () => { { type: "text", value: "Review it" }, { type: "text", value: "Sure" }, ], - output_segments: [], + expected_segments: [], reference_answer: "", guideline_paths: [guidelinePath], guideline_patterns: ["**/*.instructions.md"], @@ -143,7 +143,7 @@ describe("buildPromptInputs chatPrompt", () => { question: "placeholder", input_messages: [{ role: "user", content: "Only user" }], input_segments: [{ type: "text", value: "Only user" }], - output_segments: [], + expected_segments: [], reference_answer: "", guideline_paths: [], file_paths: [], @@ -179,7 +179,7 @@ describe("buildPromptInputs chatPrompt", () => { { type: "text", value: "" }, { type: "text", value: "Real content" }, ], - output_segments: [], + expected_segments: [], reference_answer: "", guideline_paths: [guidelinePath], guideline_patterns: ["**/*.instructions.md"], @@ -212,7 +212,7 @@ describe("buildPromptInputs chatPrompt", () => { { type: "text", value: "@[superman]: hello" }, { type: "text", value: "ack" }, ], - output_segments: [], + expected_segments: [], reference_answer: "", guideline_paths: [], file_paths: [],