diff --git a/CHANGELOG.md b/CHANGELOG.md index 02d27e1..e466131 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,44 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.6.0] - 2026-07-14 + +### Added +- Judge-specific Skills for `agent_judge` via `judge.skills`. These Skills are + installed only into the judge agent and can provide reusable grading rubrics, + evidence rules, and domain-specific constraints without leaking into the run + agent or benchmark baseline. +- Judge Skill metadata in JSON, JUnit, and HTML reports, including the source, + path, target, and callable Skill name used for the evaluation. + +### Fixed +- Judge prompts now identify installed Skills by their callable names and + explicitly require invoking each Skill tool and reading its body before + grading. Source paths remain only as an internal fallback when no name is + available. + +## [0.5.0] - 2026-07-09 + +### Added +- Configurable prompt delivery for `agent_judge` through `judge.context`. + Supports `minimal` and `standard` context profiles, per-field delivery + modes, inline-size limits, generated-file indexing, and attachment file + references. Reports include `judge_context` metadata describing how review + materials and prompts were delivered. +- Built-in CLI agents now route large prompts through runtime-readable files + and stdin when they exceed `SKILL_UP_PROMPT_INLINE_MAX_BYTES` (default + `32768` bytes), while shorter prompts remain inline. + +### Changed +- `agent_judge` now defaults to materialized review context (`profile: + standard`) instead of inlining the full transcript and workspace diff into + the judge prompt. Large artifacts are delivered through runtime-readable + files, while short final messages remain inline. Evals that require legacy + inline transcript delivery can set `judge.context.transcript: include`. + +The `v0.5.0` release tag is available at +[GitHub](https://github.com/alibaba/skill-up/releases/tag/v0.5.0). + ## [0.4.0] - 2026-07-07 ### Added @@ -22,12 +60,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `result.json`; turn-scoped failure details in JUnit XML). ### Changed -- `agent_judge` now defaults to materialized review context (`profile: - standard`) instead of inlining full transcript and workspace diff into the - judge prompt. The judge receives a materials table with file references for - large artifacts, while short final messages remain inline. Evals that require - legacy inline transcript can set `judge.context.transcript: include` subject - to `judge.context.limits.max_bytes`. - `skill-up run` now validates only the cases left after `--include-case-name` / `--exclude-case-name` filters (plus the eval-level config), so an invalid case that is filtered out no longer blocks a filtered @@ -36,14 +68,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 case) regardless of filters. ### Added -- `judge.context` for `agent_judge`, with `minimal` and `standard` profiles, - per-field delivery modes, inline-size limits, generated-file indexing, and - attachment file references. Reports now include `judge_context` metadata with - materialization and prompt-delivery details. -- Built-in CLI agents now route large prompts through prompt delivery, writing - prompt artifacts to disk and feeding runtime-readable prompt files through - stdin when the prompt exceeds `SKILL_UP_PROMPT_INLINE_MAX_BYTES` (default - 32768 bytes). - Config validation now rejects duplicate case IDs and duplicate `cases.files` references. Since reports and artifacts are keyed by case ID and one case runs per `cases.files` entry, a collision would silently overwrite results or @@ -377,6 +401,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 project and delivers the end-to-end capability to declare eval environments, run cases and emit structured reports as described in [README.md](README.md). +[0.6.0]: https://github.com/alibaba/skill-up/releases/tag/v0.6.0 +[0.5.0]: https://github.com/alibaba/skill-up/releases/tag/v0.5.0 +[0.4.0]: https://github.com/alibaba/skill-up/releases/tag/v0.4.0 [0.3.0]: https://github.com/alibaba/skill-up/releases/tag/v0.3.0 [0.2.4]: https://github.com/alibaba/skill-up/releases/tag/v0.2.4 [0.2.3]: https://github.com/alibaba/skill-up/releases/tag/v0.2.3 diff --git a/docs/design/custom-engine.md b/docs/design/custom-engine.md index c759ea6..e67fe11 100644 --- a/docs/design/custom-engine.md +++ b/docs/design/custom-engine.md @@ -464,6 +464,14 @@ The `local` transport runs a command via `runtime.Exec`. The command runs inside current runtime, so it can access the runtime workspace, installed skills, fixtures, MCP config, and environment variables. +Top-level `skills` are installed before the main run agent executes. If +`judge.type: agent_judge` declares `judge.skills`, those Skills are installed +separately before the judge agent runs. Both paths use the Agent adapter's +`InstallSkill` implementation; skill-up does not concatenate Skill files into +the prompt as a fallback. Custom and remote engines that need judge Skills must +make their agent process discover the installed Skill directory according to +their own conventions. + Example: ```yaml diff --git a/docs/guide/writing-evals.md b/docs/guide/writing-evals.md index 8a77541..dac9bf0 100644 --- a/docs/guide/writing-evals.md +++ b/docs/guide/writing-evals.md @@ -659,6 +659,9 @@ Let an LLM grade against rubric criteria — useful when semantic understanding judge: type: agent_judge model: anthropic/claude-sonnet-4-6 # Model used by the judge + skills: # Optional: judge-only Skills + - source: local_path + path: evals/fixtures/judge-rubric criteria: # Natural-language rubric - "Identifies a real bug with an accurate location" - "Does not flag correct code as a bug" @@ -667,6 +670,14 @@ judge: timeout_seconds: 60 # Optional: bound a single judge call (0 = no judge-level deadline, parent case timeout still applies) ``` +`judge.skills` is supported only for `agent_judge`. These Skills are installed +into the judge agent, not the run agent, and top-level `skills` are not +automatically installed into the judge. In benchmark mode, judge Skills are +installed for both `with_skill` and `without_skill` runs because they are +grading tooling, not the Skill under test. Installation uses each Agent +adapter's native Skill mechanism; skill-up does not concatenate Skill files +into the judge prompt. + `agent_judge` materializes review context into files and injects a small materials table into the judge prompt. When `judge.context` is omitted, the default profile is `standard`: `final_message` is included inline, while diff --git a/docs/zh/guide/writing-evals.md b/docs/zh/guide/writing-evals.md index 5cfc167..5e8d596 100644 --- a/docs/zh/guide/writing-evals.md +++ b/docs/zh/guide/writing-evals.md @@ -634,6 +634,9 @@ judge: judge: type: agent_judge model: anthropic/claude-sonnet-4-6 # 评审使用的模型 + skills: # 可选：仅供 judge 使用的 Skills + - source: local_path + path: evals/fixtures/judge-rubric criteria: # 评估标准（自然语言描述） - "输出中识别了真实存在的 bug，并给出了准确位置" - "没有将正确代码误报为 bug" @@ -642,6 +645,12 @@ judge: timeout_seconds: 60 # 可选：限制单次 judge 调用时长（0 = 不加 judge 级 deadline，仍受 case timeout 约束） ``` +`judge.skills` 仅支持 `agent_judge`。这些 Skills 会安装到 judge agent， +不会安装到主运行 agent；顶层 `skills` 也不会自动安装到 judge。开启 +benchmark 时，`with_skill` 和 `without_skill` 都会安装 judge Skills，因为它们是 +评分工具，不是被测 Skill。安装过程使用各 Agent adapter 原生的 Skill 机制； +skill-up 不会把 Skill 文件内容拼接进 judge prompt。 + `agent_judge` 会将评审上下文物化为文件，并在 judge prompt 中注入一个很小的 材料表。未配置 `judge.context` 时，默认使用 `standard` profile： `final_message` 内联，`transcript` 和 `workspace_diff` 以文件引用形式提供， diff --git a/e2e/agent_test.go b/e2e/agent_test.go index 8070f4d..5efd6a8 100644 --- a/e2e/agent_test.go +++ b/e2e/agent_test.go @@ -441,7 +441,7 @@ input: prompt: Reply with exactly the word hello. Do not run commands. constraints: - timeout_seconds: 5 + timeout_seconds: 30 max_turns: 1 expect: diff --git a/internal/agent/agent.go b/internal/agent/agent.go index ae7fb81..4587481 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -190,6 +190,11 @@ func (a *BaseAgent) Name() string { return a.Cfg.Name } +// SkillPath returns the agent's default skill installation path. +func (a *BaseAgent) SkillPath() string { + return a.Cfg.SkillPath +} + // CheckCredentials checks if the required credentials are set. func (a *BaseAgent) CheckCredentials(ctx context.Context) error { switch a.Cfg.ModelProvider { diff --git a/internal/agent/codex_test.go b/internal/agent/codex_test.go index 2504c71..6807491 100644 --- a/internal/agent/codex_test.go +++ b/internal/agent/codex_test.go @@ -353,6 +353,7 @@ func TestCodexRunTurn_FirstTurnDelegatesToRun(t *testing.T) { } if result == nil { t.Fatal("expected non-nil result") + return } // First turn should use "codex exec", not "codex resume" if containsCommand(rt.commands, "codex resume") { @@ -745,6 +746,7 @@ func TestCodexRun_PreservesArtifactsOnNonZeroExit(t *testing.T) { } if sessionResult == nil { t.Fatal("expected session result") + return } if sessionResult.ExitCode != 1 { t.Fatalf("expected exit code 1, got %d", sessionResult.ExitCode) diff --git a/internal/agent/custom_test.go b/internal/agent/custom_test.go index 502545d..5e4c747 100644 --- a/internal/agent/custom_test.go +++ b/internal/agent/custom_test.go @@ -370,6 +370,7 @@ func TestCustomAgent_RunLocal_NonZeroProcessExitFails(t *testing.T) { } if res == nil { t.Fatal("expected session result to be preserved") + return } // The real process exit code must surface in the result, not the JSON's 0. if res.ExitCode != 7 { diff --git a/internal/cli/judge_debug.go b/internal/cli/judge_debug.go index af8eee6..9feccd5 100644 --- a/internal/cli/judge_debug.go +++ b/internal/cli/judge_debug.go @@ -142,7 +142,7 @@ func runJudgeDebug(cmd *cobra.Command, args []string) error { } var input judgeDebugInput - if err := json.Unmarshal(data, &input); err != nil { + if err := json.Unmarshal(data, &input); err != nil { //nolint:musttag // debug input embeds config structs with existing tags. return fmt.Errorf("parse input JSON: %w", err) } diff --git a/internal/cli/judge_debug_test.go b/internal/cli/judge_debug_test.go index 3c7054b..ebaad67 100644 --- a/internal/cli/judge_debug_test.go +++ b/internal/cli/judge_debug_test.go @@ -25,7 +25,7 @@ import ( // writeJudgeDebugInput serialises a judgeDebugInput to a JSON file and returns the path. func writeJudgeDebugInput(t *testing.T, dir string, input judgeDebugInput) string { t.Helper() - data, err := json.Marshal(input) + data, err := json.Marshal(input) //nolint:musttag // debug input embeds config structs with existing tags. if err != nil { t.Fatalf("marshal judge debug input: %v", err) } @@ -238,6 +238,7 @@ func TestMockJudgeAgent_Run(t *testing.T) { } if sr == nil { t.Fatal("Run: nil SessionResult") + return } var parsed struct { diff --git a/internal/cli/run.go b/internal/cli/run.go index acd2716..486e5b5 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -186,10 +186,11 @@ func loadAndPrepareConfig(ctx context.Context, cmd *cobra.Command, args []string return nil, nil, nil, errors.New("no cases matched the filter") } - // Validate only the selected cases (after filters). Full-suite per-case - // validation is the job of `skill-up validate`; here an invalid case that - // was filtered out must not block the run. - if err := config.NewValidator().ValidateCases(cases); err != nil { + // Validate only the selected cases (after filters), while still applying + // eval-level judge defaults to case-level overrides. Full-suite per-case + // validation is the job of `skill-up validate`; here an invalid case that was + // filtered out must not block the run. + if err := config.NewValidator().ValidateCasesWithEvalDefaults(evalCfg, cases); err != nil { return nil, nil, nil, fmt.Errorf("validation failed: %w", err) } diff --git a/internal/config/loader_test.go b/internal/config/loader_test.go index 513125d..a6c4a9a 100644 --- a/internal/config/loader_test.go +++ b/internal/config/loader_test.go @@ -267,6 +267,54 @@ cases: } } +func TestLoader_LoadEvalConfig_JudgeSkills(t *testing.T) { + t.Parallel() + + tmp := t.TempDir() + evalPath := filepath.Join(tmp, "eval.yaml") + content := `schema_version: v1alpha1 + +environment: + type: none + +engine: + name: claude_code + +cases: + files: + - evals/cases/basic.yaml + +judge: + type: agent_judge + model: test-model + skills: + - source: local_path + path: evals/fixtures/default-judge + - source: local_path + path: evals/fixtures/security-judge + target: ~/.claude/skills/security-judge + criteria: + - criterion one +` + if err := os.WriteFile(evalPath, []byte(content), 0o600); err != nil { + t.Fatalf("failed to write temp eval.yaml: %v", err) + } + + cfg, err := NewLoader(evalPath).LoadEvalConfig() + if err != nil { + t.Fatalf("LoadEvalConfig failed: %v", err) + } + if len(cfg.Judge.Skills) != 2 { + t.Fatalf("Judge.Skills length = %d, want 2", len(cfg.Judge.Skills)) + } + if got := cfg.Judge.Skills[0].Path; got != "evals/fixtures/default-judge" { + t.Fatalf("Judge.Skills[0].Path = %q", got) + } + if got := cfg.Judge.Skills[1].Target; got != "~/.claude/skills/security-judge" { + t.Fatalf("Judge.Skills[1].Target = %q", got) + } +} + // nolint:funlen // table-driven matrix over the documented defaults; keeping // each case inline beats spreading them across helpers. func TestLoader_LoadEvalConfig_AppliesDefaults(t *testing.T) { diff --git a/internal/config/schema.go b/internal/config/schema.go index 7a12235..b249df9 100644 --- a/internal/config/schema.go +++ b/internal/config/schema.go @@ -190,6 +190,7 @@ type JudgeConfig struct { ScriptPath string `json:"script_path,omitempty" yaml:"script_path,omitempty"` Model string `json:"model,omitempty" yaml:"model,omitempty"` Criteria []string `json:"criteria,omitempty" yaml:"criteria,omitempty"` + Skills []SkillRef `json:"skills,omitempty" yaml:"skills,omitempty"` Context *JudgeContextConfig `json:"context,omitempty" yaml:"context,omitempty"` // PassThreshold is the minimum pass rate for agent_judge. // Nil means "not configured", so the judge layer applies its default of 0.7. diff --git a/internal/config/schema_test.go b/internal/config/schema_test.go index 6e546ed..c52d8b5 100644 --- a/internal/config/schema_test.go +++ b/internal/config/schema_test.go @@ -11,6 +11,7 @@ func TestDefaultEvalConfig(t *testing.T) { //nolint:cyclop,gocyclo // exhaustive cfg := DefaultEvalConfig() if cfg == nil { t.Fatal("DefaultEvalConfig() returned nil") + return } // Verify schema version diff --git a/internal/config/validator.go b/internal/config/validator.go index c4e3197..58d1ee1 100644 --- a/internal/config/validator.go +++ b/internal/config/validator.go @@ -96,7 +96,7 @@ func (v *Validator) ValidateEvalConfig(cfg *EvalConfig) error { } errs = append(errs, validateCollectArtifacts("cases.defaults.collect_artifacts", cfg.Cases.Defaults.CollectArtifacts)...) - errs = append(errs, validateJudgeContext(cfg.Judge.Context)...) + errs = append(errs, validateJudgeTypeAndLocalFields(cfg.Judge)...) if len(errs) > 0 { return fmt.Errorf("validation errors:\n - %s", strings.Join(errs, "\n - ")) @@ -143,8 +143,8 @@ func (v *Validator) ValidateCaseConfig(cfg *CaseConfig) error { // validate per-turn judge rules reference valid turn numbers turnsTotal := len(cfg.Input.Turns) - if cfg.Judge.Type != "" { - errs = append(errs, validateJudgeTypeAndFields(cfg.Judge)...) + if judgeNeedsLocalValidation(cfg.Judge) { + errs = append(errs, validateJudgeTypeAndLocalFields(cfg.Judge)...) } var allRules []Rule allRules = append(allRules, cfg.Judge.Success...) @@ -160,35 +160,56 @@ func (v *Validator) ValidateCaseConfig(cfg *CaseConfig) error { return nil } -// validateJudgeTypeAndFields validates judge type and its conditional fields. -func validateJudgeTypeAndFields(judge JudgeConfig) []string { +func judgeNeedsLocalValidation(judge JudgeConfig) bool { + return judge.Type != "" || len(judge.Skills) > 0 || judge.Context != nil +} + +// validateJudgeTypeAndLocalFields validates judge fields that do not depend on inheritance. +func validateJudgeTypeAndLocalFields(judge JudgeConfig) []string { var errs []string if judge.Type != "" && !isValidJudgeType(judge.Type) { errs = append(errs, "judge.type must be one of: rule_based, script, agent_judge") } + if len(judge.Skills) > 0 && judge.Type != judgeTypeAgentJudge { + errs = append(errs, "judge.skills is only supported when judge.type is agent_judge") + } + errs = append(errs, validateSkillRefs("judge.skills", judge.Skills)...) + + errs = append(errs, validatePassThreshold(judge.PassThreshold)...) + errs = append(errs, validateJudgeContext(judge.Context)...) + + if judge.TimeoutSeconds != nil && *judge.TimeoutSeconds < 0 { + errs = append(errs, "judge.timeout_seconds must be non-negative") + } - // script type requires script_path + return errs +} + +func validateJudgeTypeAndRequiredFields(judge JudgeConfig) []string { + errs := validateJudgeTypeAndLocalFields(judge) if judge.Type == judgeTypeScript && judge.ScriptPath == "" { errs = append(errs, "judge.script_path is required when judge.type is script") } - - // agent_judge type requires model and criteria if judge.Type == judgeTypeAgentJudge && judge.Model == "" { errs = append(errs, "judge.model is required when judge.type is agent_judge") } - if judge.Type == judgeTypeAgentJudge && len(judge.Criteria) == 0 { errs = append(errs, "judge.criteria is required when judge.type is agent_judge") } + return errs +} - errs = append(errs, validatePassThreshold(judge.PassThreshold)...) - errs = append(errs, validateJudgeContext(judge.Context)...) - - if judge.TimeoutSeconds != nil && *judge.TimeoutSeconds < 0 { - errs = append(errs, "judge.timeout_seconds must be non-negative") +func validateSkillRefs(field string, refs []SkillRef) []string { + var errs []string + for i, ref := range refs { + if strings.TrimSpace(ref.Source) == "" { + errs = append(errs, fmt.Sprintf("%s[%d].source is required", field, i)) + } + if strings.TrimSpace(ref.Path) == "" { + errs = append(errs, fmt.Sprintf("%s[%d].path is required", field, i)) + } } - return errs } @@ -293,7 +314,7 @@ func (v *Validator) ValidateAll(result *EvalResult) error { if err := v.ValidateEvalConfig(result.Eval); err != nil { return err } - return v.ValidateCases(result.Cases) + return v.ValidateCasesWithEvalDefaults(result.Eval, result.Cases) } // ValidateCases validates a set of cases: it rejects duplicate effective IDs @@ -319,6 +340,40 @@ func (v *Validator) ValidateCases(cases []*CaseConfig) error { return nil } +// ValidateCasesWithEvalDefaults validates already-loaded cases and effective +// judge settings after applying eval-level defaults. Unlike ValidateAll, it +// does not require eval.cases.files and is therefore suitable for filtered +// runs and Anthropic evals.json auto mode where cases are loaded directly. +func (v *Validator) ValidateCasesWithEvalDefaults(eval *EvalConfig, cases []*CaseConfig) error { + if err := v.ValidateCases(cases); err != nil { + return err + } + for _, c := range cases { + effectiveJudge := mergeJudgeConfigForValidation(eval.Judge, c.Judge) + if errs := validateJudgeTypeAndRequiredFields(effectiveJudge); len(errs) > 0 { + return fmt.Errorf("case %s: validation errors:\n - %s", c.ID, strings.Join(errs, "\n - ")) + } + } + return nil +} + +func mergeJudgeConfigForValidation(global, caseLevel JudgeConfig) JudgeConfig { + if caseLevel.Type != "" { + merged := caseLevel + if merged.Model == "" { + merged.Model = global.Model + } + if merged.PassThreshold == nil { + merged.PassThreshold = global.PassThreshold + } + if merged.TimeoutSeconds == nil { + merged.TimeoutSeconds = global.TimeoutSeconds + } + return merged + } + return global +} + // duplicateCaseErrors reports duplicate source-file references and duplicate // effective case IDs within the given case set. skill-up writes reports and // artifacts keyed by case ID and runs one case per cases.files entry, so a diff --git a/internal/config/validator_test.go b/internal/config/validator_test.go index 0a73d24..5871d73 100644 --- a/internal/config/validator_test.go +++ b/internal/config/validator_test.go @@ -335,7 +335,7 @@ func TestValidator_ValidateEvalConfig(t *testing.T) { errMsg: "cases.retry_policy.max_retries must be <=", }, { - name: "invalid judge.type at eval level is ignored", + name: "invalid judge.type at eval level is rejected", cfg: &EvalConfig{ SchemaVersion: "v1alpha1", Environment: Environment{Type: "none"}, @@ -353,7 +353,8 @@ func TestValidator_ValidateEvalConfig(t *testing.T) { Type: "invalid", }, }, - wantErr: false, + wantErr: true, + errMsg: "judge.type must be one of", }, { name: "valid judge context at eval level", @@ -404,7 +405,7 @@ func TestValidator_ValidateEvalConfig(t *testing.T) { errMsg: "judge.context.profile must be one of: minimal, standard", }, { - name: "script without script_path at eval level is ignored", + name: "script without script_path at eval level is checked by ValidateAll", cfg: &EvalConfig{ SchemaVersion: "v1alpha1", Environment: Environment{Type: "none"}, @@ -425,7 +426,7 @@ func TestValidator_ValidateEvalConfig(t *testing.T) { wantErr: false, }, { - name: "agent_judge without model at eval level is ignored", + name: "agent_judge without model at eval level is checked by ValidateAll", cfg: &EvalConfig{ SchemaVersion: "v1alpha1", Environment: Environment{Type: "none"}, @@ -470,7 +471,7 @@ func TestValidator_ValidateEvalConfig(t *testing.T) { wantErr: false, }, { - name: "agent_judge with threshold above one at eval level is ignored", + name: "agent_judge with threshold above one at eval level is rejected", cfg: &EvalConfig{ SchemaVersion: "v1alpha1", Environment: Environment{Type: "none"}, @@ -491,7 +492,8 @@ func TestValidator_ValidateEvalConfig(t *testing.T) { PassThreshold: float64Ptr(1.1), }, }, - wantErr: false, + wantErr: true, + errMsg: "judge.pass_threshold must be between", }, { name: "valid network_policy deny_all with opensandbox", @@ -1051,6 +1053,81 @@ func TestValidator_ValidateAll(t *testing.T) { }) } +func TestValidator_JudgeSkills(t *testing.T) { + t.Parallel() + validator := NewValidator() + + base := func(judge JudgeConfig) *EvalConfig { + return &EvalConfig{ + SchemaVersion: "v1alpha1", + Environment: Environment{Type: "none"}, + Engine: EngineConfig{Name: "claude_code"}, + Cases: CasesConfig{Files: []string{"evals/cases/test.yaml"}}, + Judge: judge, + } + } + + tests := []struct { + name string + judge JudgeConfig + errMsg string + }{ + { + name: "agent_judge accepts skills", + judge: JudgeConfig{ + Type: "agent_judge", + Model: "test-model", + Criteria: []string{"criterion"}, + Skills: []SkillRef{{Source: "local_path", Path: "evals/fixtures/judge-skill"}}, + }, + }, + { + name: "rule_based rejects skills", + judge: JudgeConfig{ + Type: "rule_based", + Skills: []SkillRef{{Source: "local_path", Path: "evals/fixtures/judge-skill"}}, + }, + errMsg: "judge.skills is only supported", + }, + { + name: "local_path requires path", + judge: JudgeConfig{ + Type: "agent_judge", + Model: "test-model", + Criteria: []string{"criterion"}, + Skills: []SkillRef{{Source: "local_path"}}, + }, + errMsg: "judge.skills[0].path is required", + }, + { + name: "non local source requires path", + judge: JudgeConfig{ + Type: "agent_judge", + Model: "test-model", + Criteria: []string{"criterion"}, + Skills: []SkillRef{{Source: "registry"}}, + }, + errMsg: "judge.skills[0].path is required", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := validator.ValidateEvalConfig(base(tt.judge)) + if tt.errMsg == "" { + if err != nil { + t.Fatalf("ValidateEvalConfig() error = %v, want nil", err) + } + return + } + if err == nil || !strings.Contains(err.Error(), tt.errMsg) { + t.Fatalf("ValidateEvalConfig() error = %v, want containing %q", err, tt.errMsg) + } + }) + } +} + // caseIDSrc pairs an effective case ID with the cases.files entry it was // loaded from, as the loader populates CaseConfig.ID / CaseConfig.SourceFile. type caseIDSrc struct { @@ -1076,7 +1153,7 @@ func TestValidator_ValidateCases_Duplicates(t *testing.T) { tests := []struct { name string cases []caseIDSrc - wantParts []string // non-empty ⇒ expect an error containing every part + wantParts []string // non-empty means expect an error containing every part }{ { name: "duplicate explicit id names both files", @@ -1084,13 +1161,11 @@ func TestValidator_ValidateCases_Duplicates(t *testing.T) { wantParts: []string{`duplicate case id "dup"`, "evals/cases/a.yaml", "evals/cases/b.yaml"}, }, { - // Files in different dirs with colliding basenames ⇒ same implicit ID. name: "duplicate filename-derived id", cases: []caseIDSrc{{"smoke", "evals/cases/smoke.yaml"}, {"smoke", "evals/full/smoke.yaml"}}, wantParts: []string{`duplicate case id "smoke"`}, }, { - // Same file, differently spelled; normalization must collapse them. name: "duplicate file reference after normalization", cases: []caseIDSrc{{"a", "evals/cases/a.yaml"}, {"a", "./evals/cases/a.yaml"}}, wantParts: []string{"cases.files lists"}, @@ -1124,10 +1199,69 @@ func TestValidator_ValidateCases_Duplicates(t *testing.T) { } } +func TestValidator_ValidateAll_CaseAgentJudgeInheritsGlobalModel(t *testing.T) { + t.Parallel() + validator := NewValidator() + + result := &EvalResult{ + Eval: &EvalConfig{ + SchemaVersion: "v1alpha1", + Environment: Environment{Type: "none"}, + Engine: EngineConfig{Name: "claude_code"}, + Cases: CasesConfig{Files: []string{"evals/cases/test.yaml"}}, + Judge: JudgeConfig{ + Type: "agent_judge", + Model: "global-model", + Criteria: []string{"global criterion"}, + }, + }, + Cases: []*CaseConfig{ + { + ID: "case-agent-judge", + Input: Input{Prompt: "test"}, + Judge: JudgeConfig{ + Type: "agent_judge", + Criteria: []string{"case criterion"}, + Skills: []SkillRef{{Source: "local_path", Path: "evals/fixtures/case-judge"}}, + }, + }, + }, + } + + if err := validator.ValidateAll(result); err != nil { + t.Fatalf("ValidateAll() error = %v, want nil", err) + } +} + +func TestValidator_ValidateAll_EffectiveAgentJudgeRequiresCriteria(t *testing.T) { + t.Parallel() + validator := NewValidator() + + result := &EvalResult{ + Eval: &EvalConfig{ + SchemaVersion: "v1alpha1", + Environment: Environment{Type: "none"}, + Engine: EngineConfig{Name: "claude_code"}, + Cases: CasesConfig{Files: []string{"evals/cases/test.yaml"}}, + Judge: JudgeConfig{ + Type: "agent_judge", + Model: "global-model", + }, + }, + Cases: []*CaseConfig{ + {ID: "case-agent-judge", Input: Input{Prompt: "test"}}, + }, + } + + err := validator.ValidateAll(result) + if err == nil || !strings.Contains(err.Error(), "judge.criteria is required") { + t.Fatalf("ValidateAll() error = %v, want missing criteria", err) + } +} + // TestValidator_ValidateCases_OnlyChecksGivenCases confirms ValidateCases // validates just the cases passed to it: an invalid case omitted from the slice -// (e.g. filtered out by `skill-up run`) does not cause a failure, while -// ValidateAll over the full set still catches it. +// does not cause a failure, while passing the invalid case still catches it. func TestValidator_ValidateCases_OnlyChecksGivenCases(t *testing.T) { t.Parallel() validator := NewValidator() @@ -1143,6 +1277,21 @@ func TestValidator_ValidateCases_OnlyChecksGivenCases(t *testing.T) { } } +func TestValidator_ValidateCasesWithEvalDefaults_AllowsLoadedCasesWithoutCaseFiles(t *testing.T) { + t.Parallel() + validator := NewValidator() + eval := DefaultEvalConfig() + eval.Cases.Files = nil + + valid := &CaseConfig{ID: "anthropic-case", Input: Input{Prompt: "hi"}} + if err := validator.ValidateCasesWithEvalDefaults(eval, []*CaseConfig{valid}); err != nil { + t.Fatalf("ValidateCasesWithEvalDefaults() error = %v, want nil", err) + } + if err := validator.ValidateAll(&EvalResult{Eval: eval, Cases: []*CaseConfig{valid}}); err == nil { + t.Fatal("ValidateAll() error = nil, want missing cases.files error") + } +} + func contains(s, substr string) bool { return len(s) >= len(substr) && (s == substr || len(s) > 0 && containsHelper(s, substr)) } diff --git a/internal/evaluator/evaluator.go b/internal/evaluator/evaluator.go index adeeb88..8d93e88 100644 --- a/internal/evaluator/evaluator.go +++ b/internal/evaluator/evaluator.go @@ -26,6 +26,7 @@ import ( "github.com/alibaba/skill-up/internal/logging" "github.com/alibaba/skill-up/internal/mcp" "github.com/alibaba/skill-up/internal/observability" + "github.com/alibaba/skill-up/internal/platform" "github.com/alibaba/skill-up/internal/runtime" "github.com/alibaba/skill-up/pkg/transcript" ) @@ -35,6 +36,8 @@ var ( agentDetectWithInitParams = agent.DetectAgentWithInitParams ) +const judgeTypeAgentJudge = "agent_judge" + // ProgressObserver receives progress notifications during evaluation. // Implementations must be safe for concurrent use. type ProgressObserver interface { @@ -89,6 +92,9 @@ type EvalResult struct { // Grading is the judge evaluation result (nil if judge was skipped). Grading *judge.Result + // JudgeSkills records judge Skills configured for agent_judge. + JudgeSkills []judge.SkillInfo + // ExpectResult is the expect pre-check result (nil if no expect block). ExpectResult *judge.ExpectResult @@ -600,8 +606,11 @@ func (e *defaultEvaluator) runJudgePhaseWithSpan( logging.DebugContextf(ctx, "Judge: case %s using %s", caseCfg.ID, judgeLabel(judgeCfg)) logging.DebugContextf(ctx, "Judge: case %s generated_files=%d workspace_diff=%t", caseCfg.ID, len(judgeInput.GeneratedFiles), judgeInput.WorkspaceDiff != "") + if judgeCfg.Type == judgeTypeAgentJudge { + result.JudgeSkills = judge.SkillInfosFromRefs(judgeCfg.Skills) + } - j, err := e.newJudgeForCase(ctx, rt, judgeCfg, runAgent) + j, err := e.newJudgeForCase(ctx, rt, configName, judgeCfg, runAgent) if err != nil { result.Status = judge.StatusError result.Error = err @@ -615,7 +624,7 @@ func (e *defaultEvaluator) runJudgePhaseWithSpan( logging.DebugContextf(ctx, "Judge: case %s has no judge configured, default PASS", caseCfg.ID) return *result } - if judgeCfg.Type == "agent_judge" { + if judgeCfg.Type == judgeTypeAgentJudge { judgeInput.ArtifactDir = e.prepareOutputDir(ctx, configName, caseCfg.ID, "judge/run") } @@ -661,6 +670,7 @@ func (e *defaultEvaluator) runJudgePhaseWithSpan( func (e *defaultEvaluator) newJudgeForCase( ctx context.Context, rt runtime.Runtime, + configName string, judgeCfg config.JudgeConfig, runAgent agent.Agent, ) (judge.Judge, error) { @@ -670,6 +680,12 @@ func (e *defaultEvaluator) newJudgeForCase( if err != nil { return nil, err } + if err := e.removeDefaultRunSkillsBeforeJudge(ctx, rt, configName, judgeCfg, runAgent); err != nil { + return nil, err + } + if err := e.installJudgeSkills(ctx, rt, judgeCfg, judgeAgent); err != nil { + return nil, err + } j, err := judge.NewJudge(judgeCfg, judgeAgent, rt) if err != nil { @@ -679,8 +695,118 @@ func (e *defaultEvaluator) newJudgeForCase( return j, nil } +func (e *defaultEvaluator) installJudgeSkills(ctx context.Context, rt runtime.Runtime, judgeCfg config.JudgeConfig, judgeAgent agent.Agent) error { + if judgeCfg.Type != judgeTypeAgentJudge || len(judgeCfg.Skills) == 0 { + return nil + } + skillDir := e.resolvedSkillDir() + for i, ref := range judgeCfg.Skills { + if strings.TrimSpace(ref.Path) == "" { + return fmt.Errorf("judge.skills[%d].path is required", i) + } + if !filepath.IsAbs(ref.Path) && strings.TrimSpace(skillDir) == "" { + return errors.New("judge.skills requires a loader or skill directory to resolve relative local paths") + } + skillCfg := resolveSkillConfig(skillDir, ref) + if err := judgeAgent.InstallSkill(ctx, rt, skillCfg); err != nil { + return fmt.Errorf("failed to install judge skill judge.skills[%d].path=%q: %w", i, ref.Path, err) + } + logging.DebugContextf(ctx, "Evaluator: judge skill installed: %s", filepath.Base(skillCfg.Source)) + } + return nil +} + +func (e *defaultEvaluator) removeDefaultRunSkillsBeforeJudge( + ctx context.Context, + rt runtime.Runtime, + configName string, + judgeCfg config.JudgeConfig, + runAgent agent.Agent, +) error { + if configName == "without_skill" || judgeCfg.Type != judgeTypeAgentJudge || len(e.evalCfg.Skills) == 0 { + return nil + } + skillPath := agentDefaultSkillPath(runAgent) + if strings.TrimSpace(skillPath) == "" { + return nil + } + skillDir := e.resolvedSkillDir() + for _, ref := range e.evalCfg.Skills { + // Only remove skill-up's default install target. Explicit targets are + // user-controlled and may intentionally be shared with other setup. + if strings.TrimSpace(ref.Target) != "" { + continue + } + skillCfg := resolveSkillConfig(skillDir, ref) + if strings.TrimSpace(skillCfg.Source) == "" { + continue + } + target := filepath.Join(skillPath, filepath.Base(skillCfg.Source)) + if !isDefaultAgentSkillTarget(skillPath, target) { + continue + } + if err := removeRuntimePath(ctx, rt, target); err != nil { + return fmt.Errorf("failed to isolate judge from run skill %q: %w", ref.Path, err) + } + logging.DebugContextf(ctx, "Evaluator: removed run skill before judge: %s", target) + } + return nil +} + +type skillPathProvider interface { + SkillPath() string +} + +func agentDefaultSkillPath(ag agent.Agent) string { + provider, ok := ag.(skillPathProvider) + if !ok { + return "" + } + return provider.SkillPath() +} + +func isDefaultAgentSkillTarget(skillPath, target string) bool { + cleanSkillPath := filepath.Clean(skillPath) + cleanTarget := filepath.Clean(target) + rel, err := filepath.Rel(cleanSkillPath, cleanTarget) + if err != nil || rel == "." || strings.HasPrefix(rel, "..") { + return false + } + return true +} + +func removeRuntimePath(ctx context.Context, rt runtime.Runtime, target string) error { + if strings.TrimSpace(target) == "" { + return nil + } + targetShell := rt.Shell() + if targetShell.Family != platform.ShellPOSIX { + return nil + } + quote, err := targetShell.Quoter() + if err != nil { + return err + } + cmd := "rm -rf -- " + quote(target) + result, err := rt.Exec(ctx, cmd, runtime.ExecOptions{}) + if err != nil { + return err + } + if result.ExitCode != 0 { + return fmt.Errorf("remove %s exited with code %d: %s", target, result.ExitCode, result.Stderr) + } + return nil +} + +func (e *defaultEvaluator) resolvedSkillDir() string { + if e.loader != nil { + return e.loader.SkillDir() + } + return e.skillDir +} + func (e *defaultEvaluator) resolveJudgeAgent(ctx context.Context, judgeCfg config.JudgeConfig, runAgent agent.Agent) (agent.Agent, error) { - if judgeCfg.Type != "agent_judge" { + if judgeCfg.Type != judgeTypeAgentJudge { return runAgent, nil } @@ -886,16 +1012,11 @@ func (e *defaultEvaluator) setupCaseEnvironment(ctx context.Context, rt runtime. if configName != "without_skill" && e.loader != nil { for _, skillRef := range e.evalCfg.Skills { - skillSourceDir := e.loader.SkillDir() - skillSource := filepath.Join(skillSourceDir, skillRef.Path) - skillCfg := runtime.SkillConfig{ - Source: skillSource, - Target: skillRef.Target, - } + skillCfg := resolveSkillConfig(e.loader.SkillDir(), skillRef) if err := ag.InstallSkill(ctx, rt, skillCfg); err != nil { return fmt.Errorf("failed to install skill %s: %w", skillRef.Path, err) } - logging.DebugContextf(ctx, "Evaluator: skill installed: %s", filepath.Base(skillSource)) + logging.DebugContextf(ctx, "Evaluator: skill installed: %s", filepath.Base(skillCfg.Source)) } } @@ -909,6 +1030,17 @@ func (e *defaultEvaluator) setupCaseEnvironment(ctx context.Context, rt runtime. return nil } +func resolveSkillConfig(skillDir string, ref config.SkillRef) runtime.SkillConfig { + source := ref.Path + if source != "" && !filepath.IsAbs(source) { + source = filepath.Join(skillDir, source) + } + return runtime.SkillConfig{ + Source: source, + Target: ref.Target, + } +} + func (e *defaultEvaluator) provisionMCPConfig() (runtime.MCPConfig, map[string]string, error) { e.mcpOnce.Do(func() { skillDir := e.skillDir @@ -963,7 +1095,7 @@ func normalizeSessionResult(sessionResult *agent.SessionResult) *agent.SessionRe } func judgeNeedsWorkspaceDiff(cfg config.JudgeConfig) bool { - return cfg.Type == "agent_judge" + return cfg.Type == judgeTypeAgentJudge } func judgeLabel(cfg config.JudgeConfig) string { diff --git a/internal/evaluator/evaluator_test.go b/internal/evaluator/evaluator_test.go index 1c7e762..d70dd7c 100644 --- a/internal/evaluator/evaluator_test.go +++ b/internal/evaluator/evaluator_test.go @@ -37,18 +37,25 @@ type mockAgent struct { output string err error credErr error + skillErr error runFunc func(ctx context.Context, rt runtime.Runtime, opts agent.ExecOptions, messages []transcript.Message) (*agent.SessionResult, error) runCall atomic.Int32 credCall atomic.Int32 installCall atomic.Int32 mcpCall atomic.Int32 skillCall atomic.Int32 + skillPath string mu sync.Mutex lastMessages []transcript.Message lastSkill runtime.SkillConfig + skills []runtime.SkillConfig } func (m *mockAgent) Name() string { return m.name } +func (m *mockAgent) SkillPath() string { + return m.skillPath +} + func (m *mockAgent) Install(_ context.Context, _ runtime.Runtime) error { m.installCall.Add(1) return nil @@ -63,8 +70,9 @@ func (m *mockAgent) InstallSkill(_ context.Context, _ runtime.Runtime, cfg runti m.skillCall.Add(1) m.mu.Lock() m.lastSkill = cfg + m.skills = append(m.skills, cfg) m.mu.Unlock() - return nil + return m.skillErr } func (m *mockAgent) Check(_ context.Context, _ runtime.Runtime) error { return nil } func (m *mockAgent) CheckCredentials(_ context.Context) error { @@ -1033,6 +1041,170 @@ func TestExecuteCase_AgentTimeoutDoesNotInvokeAgentJudge(t *testing.T) { } } +func TestExecuteCase_InstallsJudgeSkillsOnJudgeAgentOnly(t *testing.T) { + skillDir := t.TempDir() + judgeAgent := &mockAgent{ + name: "judge", + output: `{"results":[{"criterion":"uses rubric","passed":true,"evidence":"rubric applied"}]}`, + } + runAgent := &mockAgent{name: "run", output: "main response"} + + origDetect := agentDetectWithInitParams + agentDetectWithInitParams = func(_ string, _ credential.AgentInitParams, _ map[string]string) (agent.Agent, error) { + return judgeAgent, nil + } + defer func() { agentDetectWithInitParams = origDetect }() + + e := newTestEvaluator(EvalOptions{ + SkillDir: skillDir, + Agent: runAgent, + EvalCfg: &config.EvalConfig{ + Engine: config.EngineConfig{Name: "mock"}, + Judge: config.JudgeConfig{ + Type: "agent_judge", + Model: "judge-model", + Criteria: []string{"uses rubric"}, + Skills: []config.SkillRef{ + {Source: "local_path", Path: "evals/fixtures/judge-skill", Target: "~/.claude/skills/judge-skill"}, + {Source: "local_path", Path: "evals/fixtures/security-judge"}, + }, + }, + }, + }) + + result := e.executeCase( + context.Background(), + &config.CaseConfig{ID: "case-judge-skills", Input: config.Input{Prompt: "hello"}}, + "without_skill", + &mockRuntime{workspace: t.TempDir()}, + runAgent, + ) + + if result.Status != judge.StatusPass { + t.Fatalf("status = %s, err=%v", result.Status, result.Error) + } + if got := runAgent.skillCall.Load(); got != 0 { + t.Fatalf("run agent InstallSkill calls = %d, want 0", got) + } + if got := judgeAgent.skillCall.Load(); got != 2 { + t.Fatalf("judge agent InstallSkill calls = %d, want 2", got) + } + if len(judgeAgent.skills) != 2 { + t.Fatalf("judge installed skills = %#v", judgeAgent.skills) + } + wantFirst := filepath.Join(skillDir, "evals/fixtures/judge-skill") + if judgeAgent.skills[0].Source != wantFirst { + t.Fatalf("first judge skill source = %q, want %q", judgeAgent.skills[0].Source, wantFirst) + } + if judgeAgent.skills[0].Target != "~/.claude/skills/judge-skill" { + t.Fatalf("first judge skill target = %q", judgeAgent.skills[0].Target) + } + if len(result.JudgeSkills) != 2 || result.JudgeSkills[0].Path != "evals/fixtures/judge-skill" { + t.Fatalf("result JudgeSkills = %#v", result.JudgeSkills) + } + if len(judgeAgent.lastMessages) != 1 || !strings.Contains(judgeAgent.lastMessages[0].Content, "Mandatory Judge Skill Use") { + t.Fatalf("judge prompt missing mandatory skill use: %#v", judgeAgent.lastMessages) + } +} + +func TestExecuteCase_JudgeSkillInstallFailureReturnsError(t *testing.T) { + judgeAgent := &mockAgent{ + name: "judge", + skillErr: errors.New("install unsupported"), + output: `{"results":[{"criterion":"uses rubric","passed":true,"evidence":"rubric applied"}]}`, + } + runAgent := &mockAgent{name: "run", output: "main response"} + + origDetect := agentDetectWithInitParams + agentDetectWithInitParams = func(_ string, _ credential.AgentInitParams, _ map[string]string) (agent.Agent, error) { + return judgeAgent, nil + } + defer func() { agentDetectWithInitParams = origDetect }() + + e := newTestEvaluator(EvalOptions{ + SkillDir: t.TempDir(), + Agent: runAgent, + EvalCfg: &config.EvalConfig{ + Engine: config.EngineConfig{Name: "mock"}, + Judge: config.JudgeConfig{ + Type: "agent_judge", + Model: "judge-model", + Criteria: []string{"uses rubric"}, + Skills: []config.SkillRef{{Source: "local_path", Path: "evals/fixtures/judge-skill"}}, + }, + }, + }) + + result := e.executeCase( + context.Background(), + &config.CaseConfig{ID: "case-judge-skill-error", Input: config.Input{Prompt: "hello"}}, + "with_skill", + &mockRuntime{workspace: t.TempDir()}, + runAgent, + ) + + if result.Status != judge.StatusError { + t.Fatalf("status = %s, want ERROR", result.Status) + } + if result.Error == nil || !strings.Contains(result.Error.Error(), `judge.skills[0].path="evals/fixtures/judge-skill"`) { + t.Fatalf("error = %v, want judge skill path context", result.Error) + } + if got := judgeAgent.runCall.Load(); got != 0 { + t.Fatalf("judge Run calls = %d, want 0 after install failure", got) + } +} + +func TestInstallJudgeSkills_AllowsAbsolutePathWithoutSkillDir(t *testing.T) { + absSkill := filepath.Join(t.TempDir(), "judge-skill") + judgeAgent := &mockAgent{name: "judge"} + e := newTestEvaluator(EvalOptions{EvalCfg: &config.EvalConfig{}}) + + err := e.installJudgeSkills(context.Background(), &mockRuntime{workspace: t.TempDir()}, config.JudgeConfig{ + Type: "agent_judge", + Model: "judge-model", + Criteria: []string{"uses rubric"}, + Skills: []config.SkillRef{{Source: "local_path", Path: absSkill}}, + }, judgeAgent) + if err != nil { + t.Fatalf("installJudgeSkills() error = %v, want nil", err) + } + if len(judgeAgent.skills) != 1 || judgeAgent.skills[0].Source != absSkill { + t.Fatalf("judge installed skills = %#v", judgeAgent.skills) + } +} + +func TestRemoveDefaultRunSkillsBeforeJudge_RemovesOnlyDefaultTargets(t *testing.T) { + var commands []string + rt := &mockRuntime{ + workspace: t.TempDir(), + execFunc: func(_ context.Context, command string, _ runtime.ExecOptions) (runtime.ExecResult, error) { + commands = append(commands, command) + return runtime.ExecResult{ExitCode: 0}, nil + }, + } + runAgent := &mockAgent{name: "run", skillPath: ".codex/skills"} + e := newTestEvaluator(EvalOptions{ + SkillDir: t.TempDir(), + EvalCfg: &config.EvalConfig{ + Skills: []config.SkillRef{ + {Source: "local_path", Path: "skill-under-test"}, + {Source: "local_path", Path: "custom-target-skill", Target: ".codex/skills/custom"}, + }, + }, + }) + + err := e.removeDefaultRunSkillsBeforeJudge(context.Background(), rt, "with_skill", config.JudgeConfig{Type: "agent_judge"}, runAgent) + if err != nil { + t.Fatalf("removeDefaultRunSkillsBeforeJudge() error = %v", err) + } + if len(commands) != 1 { + t.Fatalf("cleanup commands = %#v, want one default-target cleanup", commands) + } + if !strings.Contains(commands[0], "rm -rf -- '.codex/skills/skill-under-test'") { + t.Fatalf("cleanup command = %q", commands[0]) + } +} + func TestExecuteCase_NoJudge_DefaultPass(t *testing.T) { e := newTestEvaluator(EvalOptions{ Agent: &mockAgent{name: "test", output: "hello world"}, diff --git a/internal/judge/agent_judge.go b/internal/judge/agent_judge.go index e5279a5..9c585ee 100644 --- a/internal/judge/agent_judge.go +++ b/internal/judge/agent_judge.go @@ -72,22 +72,34 @@ type AgentJudge struct { // deadline; the parent context still applies. TimeoutSeconds int + // JudgeSkills are installed judge Skills that must guide grading. + JudgeSkills []SkillInfo + // Context controls which evaluation materials are written, referenced, or // inlined for this agent_judge invocation. Context *config.JudgeContextConfig } // NewAgentJudge creates an AgentJudge with sensible defaults. -func NewAgentJudge(ag agent.Agent, rt runtime.Runtime, model string, criteria []string, passThreshold *float64, timeoutSeconds int) *AgentJudge { - return NewAgentJudgeWithContext(ag, rt, model, criteria, passThreshold, nil, timeoutSeconds) +func NewAgentJudge(ag agent.Agent, rt runtime.Runtime, model string, criteria []string, passThreshold *float64, timeoutSeconds int, judgeSkills ...[]SkillInfo) *AgentJudge { + return NewAgentJudgeWithContextAndSkills(ag, rt, model, criteria, passThreshold, nil, timeoutSeconds, judgeSkills...) } // NewAgentJudgeWithContext creates an AgentJudge with explicit context delivery. func NewAgentJudgeWithContext(ag agent.Agent, rt runtime.Runtime, model string, criteria []string, passThreshold *float64, contextCfg *config.JudgeContextConfig, timeoutSeconds int) *AgentJudge { + return NewAgentJudgeWithContextAndSkills(ag, rt, model, criteria, passThreshold, contextCfg, timeoutSeconds) +} + +// NewAgentJudgeWithContextAndSkills creates an AgentJudge with context delivery and judge Skills. +func NewAgentJudgeWithContextAndSkills(ag agent.Agent, rt runtime.Runtime, model string, criteria []string, passThreshold *float64, contextCfg *config.JudgeContextConfig, timeoutSeconds int, judgeSkills ...[]SkillInfo) *AgentJudge { threshold := DefaultPassThreshold if passThreshold != nil { threshold = *passThreshold } + var skills []SkillInfo + if len(judgeSkills) > 0 { + skills = judgeSkills[0] + } return &AgentJudge{ Agent: ag, Runtime: rt, @@ -95,6 +107,7 @@ func NewAgentJudgeWithContext(ag agent.Agent, rt runtime.Runtime, model string, Criteria: criteria, PassThreshold: threshold, TimeoutSeconds: timeoutSeconds, + JudgeSkills: skills, Context: contextCfg, } } @@ -118,7 +131,7 @@ func (j *AgentJudge) Evaluate(ctx context.Context, in Input) (*Result, error) { } // Build the judge prompt. - prompt := buildJudgePrompt(ctx, j.Criteria, materialized) + prompt := buildJudgePrompt(ctx, j.Criteria, materialized, j.JudgeSkills) messages := []transcript.Message{{Role: transcript.RoleUser, Content: prompt, Turn: 1}} // Get criterion results via agent.Agent. Snapshot parentCtx.Err() the @@ -426,8 +439,12 @@ func canRecoverAgentJudgeResult(err error, sessionResult *agent.SessionResult) b // --------------------------------------------------------------------------- // buildJudgePrompt constructs the system + user prompt for the judge agent. -func buildJudgePrompt(_ context.Context, criteria []string, materialized *MaterializedContext) string { +func buildJudgePrompt(_ context.Context, criteria []string, materialized *MaterializedContext, judgeSkills ...[]SkillInfo) string { var sb strings.Builder + var skills []SkillInfo + if len(judgeSkills) > 0 { + skills = judgeSkills[0] + } sb.WriteString("You are an expert evaluator for an AI agent skill evaluation.\n") sb.WriteString("You must assess the agent's output against the following criteria.\n") @@ -438,43 +455,74 @@ func buildJudgePrompt(_ context.Context, criteria []string, materialized *Materi sb.WriteString("IMPORTANT: Your response MUST be valid JSON. If any string value contains double quotes, ") sb.WriteString("you MUST escape them with a backslash (e.g. \\\"example\\\"). Do NOT use unescaped double quotes inside string values.\n\n") + appendJudgeSkillInstructions(&sb, skills) + sb.WriteString("## Criteria\n") for i, c := range criteria { fmt.Fprintf(&sb, "%d. %s\n", i+1, c) } - if materialized != nil { - sb.WriteString("\n## Review Materials\n") - sb.WriteString("Read files from this table as needed to evaluate the criteria. Evidence must be traceable to the listed material keys or inline excerpts.\n\n") - sb.WriteString("| key | mode | path | bytes | notes |\n") - sb.WriteString("| --- | --- | --- | ---: | --- |\n") - for _, m := range materialized.Materials { - notes := "" - if m.Truncated { - notes = "inline excerpt truncated; full text is at path" - } - fmt.Fprintf(&sb, "| %s | %s | %s | %d | %s |\n", materialLabel(m), m.Mode, materialPromptPath(m), m.OriginalBytes, notes) + appendReviewMaterials(&sb, materialized) + appendRequiredResponseFormat(&sb, criteria) + return sb.String() +} + +func appendJudgeSkillInstructions(sb *strings.Builder, skills []SkillInfo) { + if len(skills) == 0 { + return + } + sb.WriteString("## Mandatory Judge Skill Use\n") + sb.WriteString("Before evaluating the case, you MUST invoke the Skill tool for EACH installed judge Skill below using its callable skill name and read the full Skill body. ") + sb.WriteString("Do not grade the case until you have loaded every listed judge Skill. ") + sb.WriteString("Do not grade this case using only the inline criteria. The inline criteria identify result dimensions, while the judge Skill(s) define detailed rubric, constraints, and evidence rules. ") + sb.WriteString("If an inline criterion conflicts with a judge Skill, follow the judge Skill unless the criterion defines a more specific case-level acceptance condition.\n\n") + sb.WriteString("Installed judge Skill(s):\n") + for _, skill := range skills { + fmt.Fprintf(sb, "- invoke Skill tool with name %q", skillIdentifier(skill)) + if skill.Target != "" { + fmt.Fprintf(sb, " (target: %s)", skill.Target) } - for _, m := range materialized.Materials { - if strings.TrimSpace(m.InlineContent) == "" { - continue - } - fmt.Fprintf(&sb, "\n### Inline Material: %s\n", materialLabel(m)) - if m.Truncated { - sb.WriteString("The excerpt below is truncated; read the full file at the path above if needed.\n") - } - sb.WriteString("```\n") - sb.WriteString(m.InlineContent) - sb.WriteString("\n```\n") + sb.WriteString("\n") + } + sb.WriteString("\n") +} + +func appendReviewMaterials(sb *strings.Builder, materialized *MaterializedContext) { + if materialized == nil { + return + } + sb.WriteString("\n## Review Materials\n") + sb.WriteString("Read files from this table as needed to evaluate the criteria. Evidence must be traceable to the listed material keys or inline excerpts.\n\n") + sb.WriteString("| key | mode | path | bytes | notes |\n") + sb.WriteString("| --- | --- | --- | ---: | --- |\n") + for _, m := range materialized.Materials { + notes := "" + if m.Truncated { + notes = "inline excerpt truncated; full text is at path" } + fmt.Fprintf(sb, "| %s | %s | %s | %d | %s |\n", materialLabel(m), m.Mode, materialPromptPath(m), m.OriginalBytes, notes) } + for _, m := range materialized.Materials { + if strings.TrimSpace(m.InlineContent) == "" { + continue + } + fmt.Fprintf(sb, "\n### Inline Material: %s\n", materialLabel(m)) + if m.Truncated { + sb.WriteString("The excerpt below is truncated; read the full file at the path above if needed.\n") + } + sb.WriteString("```\n") + sb.WriteString(m.InlineContent) + sb.WriteString("\n```\n") + } +} +func appendRequiredResponseFormat(sb *strings.Builder, criteria []string) { sb.WriteString("\n## Required Response Format (JSON)\n") sb.WriteString("```json\n") sb.WriteString("{\n") sb.WriteString(" \"results\": [\n") for i, c := range criteria { - fmt.Fprintf(&sb, " {\"criterion\": %q, \"passed\": true|false, \"evidence\": \"...\"}", c) + fmt.Fprintf(sb, " {\"criterion\": %q, \"passed\": true|false, \"evidence\": \"...\"}", c) if i < len(criteria)-1 { sb.WriteString(",") } @@ -483,8 +531,16 @@ func buildJudgePrompt(_ context.Context, criteria []string, materialized *Materi sb.WriteString(" ]\n") sb.WriteString("}\n") sb.WriteString("```\n") +} - return sb.String() +func skillIdentifier(skill SkillInfo) string { + if skill.Name != "" { + return skill.Name + } + if skill.Path != "" { + return skill.Path + } + return skill.Source } func materialLabel(m ContextMaterial) string { diff --git a/internal/judge/agent_judge_test.go b/internal/judge/agent_judge_test.go index 9a245e8..a0bb610 100644 --- a/internal/judge/agent_judge_test.go +++ b/internal/judge/agent_judge_test.go @@ -375,6 +375,97 @@ func TestBuildJudgePrompt_ContainsAllParts(t *testing.T) { } } +func TestBuildJudgePromptWithSkills_RequiresJudgeSkillUse(t *testing.T) { + materialized := &MaterializedContext{ + Materials: []ContextMaterial{ + { + ContextMaterialManifest: ContextMaterialManifest{ + Key: "final_message", + Mode: "include", + OriginalBytes: len("Agent final message"), + }, + InlineContent: "Agent final message", + }, + }, + } + prompt := buildJudgePrompt( + context.Background(), + []string{"criterion A"}, + materialized, + []SkillInfo{ + {Source: "local_path", Path: "evals/fixtures/judge-skill", Target: "~/.claude/skills/judge-skill", Name: "judge-skill"}, + {Source: "local_path", Path: "evals/fixtures/security-judge", Name: "security-judge"}, + }, + ) + + checks := []string{ + "## Mandatory Judge Skill Use", + "MUST invoke the Skill tool for EACH installed judge Skill", + `invoke Skill tool with name "judge-skill"`, + `invoke Skill tool with name "security-judge"`, + "read the full Skill body", + "target: ~/.claude/skills/judge-skill", + } + for _, c := range checks { + if !strings.Contains(prompt, c) { + t.Fatalf("prompt missing %q:\n%s", c, prompt) + } + } + if strings.Contains(prompt, "evals/fixtures/judge-skill") || strings.Contains(prompt, "evals/fixtures/security-judge") { + t.Fatalf("prompt should use callable skill names instead of source paths, got:\n%s", prompt) + } +} + +func TestSkillIdentifier_PrefersCallableName(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + skill SkillInfo + want string + }{ + {name: "callable name", skill: SkillInfo{Name: "judge-skill", Path: "evals/fixtures/judge-skill", Source: "source"}, want: "judge-skill"}, + {name: "path fallback", skill: SkillInfo{Path: "evals/fixtures/judge-skill", Source: "source"}, want: "evals/fixtures/judge-skill"}, + {name: "source fallback", skill: SkillInfo{Source: "source"}, want: "source"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if got := skillIdentifier(tt.skill); got != tt.want { + t.Fatalf("skillIdentifier() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestAgentJudge_EvaluatePassesJudgeSkillsInPrompt(t *testing.T) { + output := buildMockAgentOutput([]CriterionResult{ + {Criterion: "criterion A", Passed: true, Evidence: "used rubric"}, + }) + ag := &mockJudgeTestAgent{output: output} + rt := &mockJudgeTestRuntime{} + + j := NewAgentJudge( + ag, + rt, + "test-model", + []string{"criterion A"}, + nil, + 0, + []SkillInfo{{Source: "local_path", Path: "evals/fixtures/judge-skill", Name: "judge-skill"}}, + ) + _, err := j.Evaluate(context.Background(), Input{FinalMessage: "test"}) + assertNoError(t, err) + + if len(ag.lastMessages) != 1 { + t.Fatalf("lastMessages length = %d, want 1", len(ag.lastMessages)) + } + if !strings.Contains(ag.lastMessages[0].Content, `invoke Skill tool with name "judge-skill"`) { + t.Fatalf("judge prompt missing callable skill name:\n%s", ag.lastMessages[0].Content) + } +} + func TestBuildContextMetadata_UsesManifestMaterializedDir(t *testing.T) { materialized := &MaterializedContext{ Dir: "/tmp/skill-up/run/context", diff --git a/internal/judge/factory.go b/internal/judge/factory.go index 45e0ffd..47ae6e3 100644 --- a/internal/judge/factory.go +++ b/internal/judge/factory.go @@ -38,7 +38,7 @@ func NewJudge(cfg config.JudgeConfig, ag agent.Agent, rt runtime.Runtime) (Judge if ag == nil { return nil, errors.New("agent_judge requires an Agent") } - return NewAgentJudgeWithContext(ag, rt, cfg.Model, cfg.Criteria, cfg.PassThreshold, cfg.Context, derefInt(cfg.TimeoutSeconds)), nil + return NewAgentJudgeWithContextAndSkills(ag, rt, cfg.Model, cfg.Criteria, cfg.PassThreshold, cfg.Context, derefInt(cfg.TimeoutSeconds), SkillInfosFromRefs(cfg.Skills)), nil case "": // No judge configured — return nil, caller should handle diff --git a/internal/judge/factory_test.go b/internal/judge/factory_test.go index 068467c..c86a21c 100644 --- a/internal/judge/factory_test.go +++ b/internal/judge/factory_test.go @@ -98,6 +98,26 @@ func TestNewJudge_AgentJudge(t *testing.T) { } } +func TestNewJudge_AgentJudge_PropagatesJudgeSkills(t *testing.T) { + cfg := config.JudgeConfig{ + Type: "agent_judge", + Model: "test-model", + Criteria: []string{"c1"}, + Skills: []config.SkillRef{{Source: "local_path", Path: "evals/fixtures/judge-skill", Target: "~/.claude/skills/judge-skill"}}, + } + j, err := NewJudge(cfg, &mockJudgeTestAgent{}, &mockJudgeTestRuntime{}) + if err != nil { + t.Fatalf("NewJudge() error = %v", err) + } + aj, ok := j.(*AgentJudge) + if !ok { + t.Fatalf("expected *AgentJudge, got %T", j) + } + if len(aj.JudgeSkills) != 1 || aj.JudgeSkills[0].Path != "evals/fixtures/judge-skill" { + t.Fatalf("JudgeSkills = %#v", aj.JudgeSkills) + } +} + func TestNewJudge_AgentJudge_CustomThreshold(t *testing.T) { threshold := 0.9 cfg := config.JudgeConfig{ @@ -215,6 +235,7 @@ func TestMergeJudgeConfig_CaseOverridesGlobal(t *testing.T) { Model: "global-model", PassThreshold: &globalThreshold, Criteria: []string{"global-c"}, + Skills: []config.SkillRef{{Source: "local_path", Path: "evals/fixtures/global-judge"}}, TimeoutSeconds: intPtr(60), } caseLevel := config.JudgeConfig{ @@ -234,6 +255,31 @@ func TestMergeJudgeConfig_CaseOverridesGlobal(t *testing.T) { if merged.TimeoutSeconds == nil || *merged.TimeoutSeconds != 60 { t.Errorf("expected timeout 60 from global, got %v", merged.TimeoutSeconds) } + if len(merged.Skills) != 0 { + t.Errorf("expected case override to drop global skills, got %#v", merged.Skills) + } +} + +func TestMergeJudgeConfig_CaseAgentJudgeOverridesSkills(t *testing.T) { + global := config.JudgeConfig{ + Type: "agent_judge", + Model: "global-model", + Criteria: []string{"global-c"}, + Skills: []config.SkillRef{{Source: "local_path", Path: "evals/fixtures/global-judge"}}, + } + caseLevel := config.JudgeConfig{ + Type: "agent_judge", + Criteria: []string{"case-c"}, + Skills: []config.SkillRef{{Source: "local_path", Path: "evals/fixtures/case-judge"}}, + } + + merged := MergeJudgeConfig(global, caseLevel) + if merged.Model != "global-model" { + t.Fatalf("expected inherited model, got %q", merged.Model) + } + if len(merged.Skills) != 1 || merged.Skills[0].Path != "evals/fixtures/case-judge" { + t.Fatalf("expected case skills only, got %#v", merged.Skills) + } } func TestMergeJudgeConfig_CaseTimeoutOverridesGlobal(t *testing.T) { diff --git a/internal/judge/helpers_test.go b/internal/judge/helpers_test.go index 8f25c65..2a8161a 100644 --- a/internal/judge/helpers_test.go +++ b/internal/judge/helpers_test.go @@ -79,6 +79,7 @@ type mockJudgeTestAgent struct { // TimeoutSeconds correctly. ok mirrors ctx.Deadline()'s second return. observedDeadline time.Time observedDeadlineOK bool + lastMessages []transcript.Message } func (m *mockJudgeTestAgent) Name() string { return "mock-judge-agent" } @@ -99,8 +100,9 @@ func (m *mockJudgeTestAgent) Check(_ context.Context, _ runtime.Runtime) error { func (m *mockJudgeTestAgent) CheckCredentials(_ context.Context) error { return nil } -func (m *mockJudgeTestAgent) Run(ctx context.Context, _ runtime.Runtime, _ agent.ExecOptions, _ []transcript.Message) (*agent.SessionResult, error) { +func (m *mockJudgeTestAgent) Run(ctx context.Context, _ runtime.Runtime, _ agent.ExecOptions, messages []transcript.Message) (*agent.SessionResult, error) { m.observedDeadline, m.observedDeadlineOK = ctx.Deadline() + m.lastMessages = messages if m.runDelay > 0 { select { case <-ctx.Done(): diff --git a/internal/judge/judge_skill.go b/internal/judge/judge_skill.go new file mode 100644 index 0000000..3ce760c --- /dev/null +++ b/internal/judge/judge_skill.go @@ -0,0 +1,41 @@ +package judge + +import ( + "path/filepath" + + "github.com/alibaba/skill-up/internal/config" +) + +// SkillInfo describes a judge Skill configured for agent_judge. +type SkillInfo struct { + Source string `json:"source,omitempty"` + Path string `json:"path,omitempty"` + Target string `json:"target,omitempty"` + Name string `json:"name,omitempty"` +} + +// SkillInfosFromRefs converts configured Skill refs into report-safe metadata. +func SkillInfosFromRefs(refs []config.SkillRef) []SkillInfo { + if len(refs) == 0 { + return nil + } + infos := make([]SkillInfo, 0, len(refs)) + for _, ref := range refs { + name := skillInfoName(ref.Path) + infos = append(infos, SkillInfo{ + Source: ref.Source, + Path: ref.Path, + Target: ref.Target, + Name: name, + }) + } + return infos +} + +func skillInfoName(path string) string { + clean := filepath.Clean(path) + if clean == "" || clean == "." { + return "" + } + return filepath.Base(clean) +} diff --git a/internal/judge/judge_skill_test.go b/internal/judge/judge_skill_test.go new file mode 100644 index 0000000..cf151eb --- /dev/null +++ b/internal/judge/judge_skill_test.go @@ -0,0 +1,27 @@ +package judge + +import ( + "testing" + + "github.com/alibaba/skill-up/internal/config" +) + +func TestSkillInfosFromRefs_OmitsDotName(t *testing.T) { + t.Parallel() + + infos := SkillInfosFromRefs([]config.SkillRef{ + {Source: "local_path"}, + {Source: "local_path", Path: "."}, + {Source: "local_path", Path: "evals/fixtures/judge-skill"}, + }) + + if len(infos) != 3 { + t.Fatalf("SkillInfosFromRefs() returned %d infos, want 3", len(infos)) + } + if infos[0].Name != "" || infos[1].Name != "" { + t.Fatalf("dot or empty paths should not derive names: %#v", infos) + } + if infos[2].Name != "judge-skill" { + t.Fatalf("third Name = %q, want judge-skill", infos[2].Name) + } +} diff --git a/internal/platform/platform_test.go b/internal/platform/platform_test.go index defa770..94e8352 100644 --- a/internal/platform/platform_test.go +++ b/internal/platform/platform_test.go @@ -17,6 +17,7 @@ func TestHost(t *testing.T) { cmd := shell.Cmd(context.Background(), "echo hi") if cmd == nil { t.Fatal("HostShell.Cmd returned nil") + return } if cmd.Path == "" { t.Fatal("HostShell.Cmd produced a command with no executable path") diff --git a/internal/report/html.go b/internal/report/html.go index bb59881..37cc1c0 100644 --- a/internal/report/html.go +++ b/internal/report/html.go @@ -79,19 +79,20 @@ type embeddedSummary struct { } type embeddedCase struct { - ID string `json:"id"` - Title string `json:"title,omitempty"` - Status string `json:"status"` - DurationMs int64 `json:"duration_ms"` - Duration string `json:"duration"` - Turns int `json:"turns"` - Error string `json:"error,omitempty"` - Grading *embeddedGrading `json:"grading,omitempty"` - Configuration string `json:"configuration,omitempty"` - Prompt string `json:"prompt,omitempty"` - Response string `json:"response,omitempty"` - Baseline *embeddedCase `json:"baseline,omitempty"` - TurnResults []embeddedTurn `json:"turn_results,omitempty"` + ID string `json:"id"` + Title string `json:"title,omitempty"` + Status string `json:"status"` + DurationMs int64 `json:"duration_ms"` + Duration string `json:"duration"` + Turns int `json:"turns"` + Error string `json:"error,omitempty"` + Grading *embeddedGrading `json:"grading,omitempty"` + Configuration string `json:"configuration,omitempty"` + Prompt string `json:"prompt,omitempty"` + Response string `json:"response,omitempty"` + Baseline *embeddedCase `json:"baseline,omitempty"` + TurnResults []embeddedTurn `json:"turn_results,omitempty"` + JudgeSkills []judge.SkillInfo `json:"judge_skills,omitempty"` } // embeddedTurn holds per-turn data for the HTML report JavaScript. @@ -146,6 +147,7 @@ func caseResultToEmbeddedCase(cr CaseResult) embeddedCase { Prompt: cr.Prompt, Response: cr.Response, TurnResults: caseTurnResultsToEmbedded(cr.TurnResults), + JudgeSkills: cr.JudgeSkills, } if cr.Grading != nil { eg := &embeddedGrading{ diff --git a/internal/report/junit.go b/internal/report/junit.go index 73eb05c..9382fc7 100644 --- a/internal/report/junit.go +++ b/internal/report/junit.go @@ -5,6 +5,7 @@ import ( "encoding/xml" "fmt" "io" + "strconv" "strings" "time" @@ -63,13 +64,23 @@ type junitTestSuite struct { // junitTestCase is a element. type junitTestCase struct { - XMLName xml.Name `xml:"testcase"` - Name string `xml:"name,attr"` - ClassName string `xml:"classname,attr"` - Time string `xml:"time,attr"` - Failure *junitFailure `xml:"failure,omitempty"` - Error *junitError `xml:"error,omitempty"` - Skipped *junitSkipped `xml:"skipped,omitempty"` + XMLName xml.Name `xml:"testcase"` + Name string `xml:"name,attr"` + ClassName string `xml:"classname,attr"` + Time string `xml:"time,attr"` + Properties *junitProperties `xml:"properties,omitempty"` + Failure *junitFailure `xml:"failure,omitempty"` + Error *junitError `xml:"error,omitempty"` + Skipped *junitSkipped `xml:"skipped,omitempty"` +} + +type junitProperties struct { + Properties []junitProperty `xml:"property"` +} + +type junitProperty struct { + Name string `xml:"name,attr"` + Value string `xml:"value,attr"` } // junitFailure is a element. @@ -109,6 +120,7 @@ func (r *JUnitReporter) buildTestSuite(in Input) junitTestSuites { ClassName: in.SkillName, Time: fmt.Sprintf("%.3f", float64(cr.DurationMs)/1000.0), } + tc.Properties = buildJudgeSkillProperties(cr) switch cr.Status { case judge.StatusFail: @@ -157,6 +169,22 @@ func (r *JUnitReporter) buildTestSuite(in Input) junitTestSuites { return junitTestSuites{Suites: []junitTestSuite{suite}} } +func buildJudgeSkillProperties(cr CaseResult) *junitProperties { + props := []junitProperty{ + {Name: "judge.skills.count", Value: strconv.Itoa(len(cr.JudgeSkills))}, + } + for i, skill := range cr.JudgeSkills { + prefix := fmt.Sprintf("judge.skills.%d.", i) + props = append(props, + junitProperty{Name: prefix + "source", Value: skill.Source}, + junitProperty{Name: prefix + "path", Value: skill.Path}, + junitProperty{Name: prefix + "target", Value: skill.Target}, + junitProperty{Name: prefix + "name", Value: skill.Name}, + ) + } + return &junitProperties{Properties: props} +} + // buildFailureBody extracts failed assertion details for the body. // Turn-scoped assertions already include turn numbers in their text field, // making CI failure output directly actionable. diff --git a/internal/report/reporter.go b/internal/report/reporter.go index 3b19064..4e1de87 100644 --- a/internal/report/reporter.go +++ b/internal/report/reporter.go @@ -55,19 +55,20 @@ func (in Input) OverallPassRate() float64 { // CaseResult represents the result of a single case execution. type CaseResult struct { - CaseID string `json:"case_id"` - Title string `json:"title"` - Status judge.Status `json:"status"` - DurationMs int64 `json:"duration_ms"` - Turns int `json:"turns"` - InputTokens int `json:"input_tokens"` - OutputTokens int `json:"output_tokens"` - Error string `json:"error,omitempty"` - Grading *judge.Result `json:"grading"` - Configuration string `json:"configuration,omitempty"` // "with_skill" or "without_skill" - Prompt string `json:"prompt,omitempty"` // input prompt sent to the agent - Response string `json:"response,omitempty"` // agent final message - TurnResults []CaseTurnResult `json:"turn_results,omitempty"` // per-turn outcomes; nil for single-turn + CaseID string `json:"case_id"` + Title string `json:"title"` + Status judge.Status `json:"status"` + DurationMs int64 `json:"duration_ms"` + Turns int `json:"turns"` + InputTokens int `json:"input_tokens"` + OutputTokens int `json:"output_tokens"` + Error string `json:"error,omitempty"` + Grading *judge.Result `json:"grading"` + JudgeSkills []judge.SkillInfo `json:"judge_skills,omitempty"` + Configuration string `json:"configuration,omitempty"` // "with_skill" or "without_skill" + Prompt string `json:"prompt,omitempty"` // input prompt sent to the agent + Response string `json:"response,omitempty"` // agent final message + TurnResults []CaseTurnResult `json:"turn_results,omitempty"` // per-turn outcomes; nil for single-turn } // CaseTurnResult holds the outcome of a single turn for reporting purposes. diff --git a/internal/report/reporter_test.go b/internal/report/reporter_test.go index fe7aef9..b8ad38c 100644 --- a/internal/report/reporter_test.go +++ b/internal/report/reporter_test.go @@ -31,6 +31,9 @@ func sampleInput() Input { Status: judge.StatusPass, DurationMs: 45200, Turns: 5, + JudgeSkills: []judge.SkillInfo{ + {Source: "local_path", Path: "evals/fixtures/judge-skill", Target: "~/.claude/skills/judge-skill", Name: "judge-skill"}, + }, Grading: &judge.Result{ Status: judge.StatusPass, TurnsExecuted: 5, @@ -115,6 +118,9 @@ func TestJSONReporter_Write(t *testing.T) { if parsed.CaseResults[1].Status != judge.StatusFail { t.Fatalf("expected second case FAIL, got %s", parsed.CaseResults[1].Status) } + if len(parsed.CaseResults[0].JudgeSkills) != 1 || parsed.CaseResults[0].JudgeSkills[0].Path != "evals/fixtures/judge-skill" { + t.Fatalf("judge_skills not preserved in JSON: %#v", parsed.CaseResults[0].JudgeSkills) + } } func TestJSONReporter_ContainsAssertions(t *testing.T) { @@ -174,6 +180,12 @@ func TestJUnitReporter_Write(t *testing.T) { if !strings.Contains(content, `errors="1"`) { t.Fatal("expected 1 error") } + if !strings.Contains(content, `name="judge.skills.count" value="1"`) { + t.Fatal("junit should include judge skill count property") + } + if !strings.Contains(content, `name="judge.skills.0.path" value="evals/fixtures/judge-skill"`) { + t.Fatal("junit should include judge skill path property") + } } func TestJUnitReporter_FailureDetails(t *testing.T) { @@ -232,6 +244,9 @@ func TestHTMLReporter_Write(t *testing.T) { if !strings.Contains(content, "edge-case-null") { t.Fatal("missing failed case ID") } + if !strings.Contains(content, "evals/fixtures/judge-skill") { + t.Fatal("missing judge skill path in embedded report data") + } } func TestHTMLReporter_ContainsAssertionDetails(t *testing.T) { diff --git a/internal/runner/runner.go b/internal/runner/runner.go index f8ba345..af1e347 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -446,6 +446,7 @@ func evalResultToCaseResult(res *evaluator.EvalResult) report.CaseResult { InputTokens: res.InputTokens, OutputTokens: res.OutputTokens, Grading: res.Grading, + JudgeSkills: res.JudgeSkills, Configuration: res.Configuration, Prompt: res.Prompt, Response: responseContent(res), diff --git a/skills/skill-upper/references/eval-yaml.md b/skills/skill-upper/references/eval-yaml.md index efc3034..43815f1 100644 --- a/skills/skill-upper/references/eval-yaml.md +++ b/skills/skill-upper/references/eval-yaml.md @@ -42,6 +42,15 @@ cases: max_retries: 1 retry_on: [timeout, error] +judge: + type: agent_judge + model: anthropic/claude-sonnet-4-6 + skills: # 可选：仅安装给 judge agent 的评分 Skill + - source: local_path + path: evals/fixtures/judge-rubric + criteria: + - "按 judge-rubric 中的细则判断输出是否满足要求" + benchmark: enabled: false @@ -54,6 +63,8 @@ report: `collect_artifacts`（`cases.defaults` 级，或单个 `case.yaml` 内追加）用 [doublestar](https://github.com/bmatcuk/doublestar) glob（`*` 单层、`**` 跨目录）声明要采集的 workspace 文件。无论 Agent 成功/失败/超时，命中文件都会保留相对路径下载到 `///outputs/workspace/`。两层按并集去重合并。它与 `report.artifacts`（产物*类型*）、`agent_judge` 的 git diff（字符串）正交。 +`judge.skills` 仅支持 `judge.type: agent_judge`，用于给 judge agent 安装可复用的评分 Rubric Skill。它不会安装到主运行 agent；顶层 `skills` 也不会自动安装到 judge。benchmark 下 `with_skill` / `without_skill` 都会安装 judge Skills，因为它们属于评分工具。路径相对 Skill 根目录解析，安装依赖具体 Agent adapter 的原生 Skill 支持；不要把 Skill 文件内容复制进 `criteria`。 + ## 运行环境 | type | 适用场景 | 说明 | diff --git a/skills/skill-upper/references/judge-types.md b/skills/skill-upper/references/judge-types.md index 84357a5..35d4a82 100644 --- a/skills/skill-upper/references/judge-types.md +++ b/skills/skill-upper/references/judge-types.md @@ -45,8 +45,11 @@ judge: judge: type: agent_judge model: anthropic/claude-sonnet-4-6 + skills: + - source: local_path + path: evals/fixtures/judge-rubric criteria: - - "输出中识别了真实存在的 bug，并给出了准确位置" + - "输出中识别了真实存在的 bug，并符合 judge-rubric 中的评分细则" - "没有将正确代码误报为 bug" - "建议具有可操作性，不是泛泛而谈" pass_threshold: 0.7 @@ -57,6 +60,8 @@ judge: - 会额外消耗 token，慢且贵 - criteria 尽量具体、可验证 - 能拆出确定性条件时先用 `rule_based` / `expect` 挡一道 +- 需要长 Rubric、领域规则、复用评分规范时，把它们放进 `judge.skills` +- `judge.skills` 只会安装给 judge agent，不会污染被测 run agent；安装依赖具体 Agent adapter 的 Skill 支持，不会回退为 prompt 拼接 ## script — 自定义脚本