diff --git a/CHANGELOG.md b/CHANGELOG.md index 092c8af..5ad0f03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `agent_judge` now uses stable criterion IDs and a strict JSON response contract, rejects malformed or semantically incomplete judge output, and always derives report labels and ordering from configured criteria. +- `agent_judge` now makes one bounded output-correction attempt after strict + decoding or semantic validation fails. Both raw responses are preserved, + retry engine artifacts are isolated under `outputs/judge/run/retry/`, and + runtime-backed artifacts are snapshotted before a retry can overwrite them. + Independent correction runs begin at turn one, while aggregate Judge duration + and usage metrics cover the complete correction flow without changing report + schemas. - HTML and Markdown reports now identify the top-level duration as evaluation wall time and show per-case tested-agent execution time plus input, output, and total token usage. Benchmark cases include compact with-Skill, diff --git a/e2e/agent_test.go b/e2e/agent_test.go index 6c59655..825fde9 100644 --- a/e2e/agent_test.go +++ b/e2e/agent_test.go @@ -1581,6 +1581,14 @@ judge: if strings.TrimSpace(grading.Expectations[0].Evidence) == "" { t.Fatalf("expected non-empty judge evidence: %s", data) } + rawResponsePath := filepath.Join(outputDir, "iteration-1", "qoder-contract", "with_skill", "outputs", "judge", "run", "raw-response-attempt-1.txt") + rawResponse, err := os.ReadFile(rawResponsePath) + if err != nil { + t.Fatalf("read Qoder Judge raw response artifact: %v", err) + } + if strings.TrimSpace(string(rawResponse)) == "" { + t.Fatalf("Qoder Judge raw response artifact is empty: %s", rawResponsePath) + } } // TestAgent_QoderCLI_NoneRuntime_FullRun tests qodercli agent with none runtime. diff --git a/e2e/pipeline_test.go b/e2e/pipeline_test.go index d1e7e44..f207a33 100644 --- a/e2e/pipeline_test.go +++ b/e2e/pipeline_test.go @@ -104,6 +104,117 @@ func TestPipeline_FullRun_WithMockEngine(t *testing.T) { } } +// TestPipeline_AgentJudgeCorrectionRetry verifies the bounded correction path +// through the real CLI pipeline with deterministic engine output. +func TestPipeline_AgentJudgeCorrectionRetry(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + stateFile := filepath.Join(t.TempDir(), "judge-attempts.txt") + outputDir := t.TempDir() + writeFile(t, filepath.Join(dir, "SKILL.md"), "# Judge correction fixture\n") + writeFile(t, filepath.Join(dir, "evals", "eval.yaml"), `schema_version: v1alpha1 + +environment: + type: none + +skills: + - source: local_path + path: . + +engine: + name: qoder-cli + model: + provider: qoder + name: auto + +cases: + files: + - evals/cases/correction.yaml + defaults: + timeout_seconds: 30 + max_turns: 1 + parallelism: 1 + +report: + formats: [json] +`) + criterion := "The final response contains the marker MOCK_CORRECTION_MARKER." + writeFile(t, filepath.Join(dir, "evals", "cases", "correction.yaml"), `id: judge-correction +title: Agent judge correction retry +input: + prompt: Return the marker MOCK_CORRECTION_MARKER. +constraints: + timeout_seconds: 30 + max_turns: 1 +judge: + type: agent_judge + model: auto + criteria: + - "The final response contains the marker MOCK_CORRECTION_MARKER." + pass_threshold: 1.0 +`) + + result := Run(t, RunConfig{ + Env: mockEngineEnv(t, + "MOCK_JUDGE_CORRECTION_RETRY=1", + "MOCK_JUDGE_STATE_FILE="+stateFile, + ), + WorkDir: dir, + Timeout: 60 * time.Second, + }, "run", filepath.Join(dir, "evals", "eval.yaml"), "--output-dir", outputDir, "--verbose") + if result.ExitCode != 0 { + t.Fatalf("correction retry run failed: exit=%d\nstdout=%s\nstderr=%s", result.ExitCode, result.Stdout, result.Stderr) + } + + state, err := os.ReadFile(stateFile) + if err != nil { + t.Fatalf("read Judge attempt state: %v", err) + } + if string(state) != "initial\ncorrection\n" { + t.Fatalf("expected exactly one correction retry, got attempts %q", state) + } + + caseDir := filepath.Join(outputDir, "iteration-1", "judge-correction", "with_skill") + gradingData, err := os.ReadFile(filepath.Join(caseDir, "grading.json")) + if err != nil { + t.Fatalf("read grading.json: %v", err) + } + var grading struct { + Expectations []struct { + Text string `json:"text"` + Passed bool `json:"passed"` + Evidence string `json:"evidence"` + } `json:"expectations"` + Summary struct { + PassRate float64 `json:"pass_rate"` + } `json:"summary"` + } + if err := json.Unmarshal(gradingData, &grading); err != nil { + t.Fatalf("parse grading.json: %v", err) + } + if grading.Summary.PassRate != 1 || len(grading.Expectations) != 1 || !grading.Expectations[0].Passed { + t.Fatalf("expected corrected Judge PASS: %s", gradingData) + } + if grading.Expectations[0].Text != criterion || strings.TrimSpace(grading.Expectations[0].Evidence) == "" { + t.Fatalf("unexpected corrected expectation: %#v", grading.Expectations) + } + + judgeDir := filepath.Join(caseDir, "outputs", "judge", "run") + for _, relativePath := range []string{ + "stdout.json", + "raw-response-attempt-1.txt", + filepath.Join("retry", "stdout.json"), + "raw-response-attempt-2.txt", + } { + path := filepath.Join(judgeDir, relativePath) + info, err := os.Stat(path) + if err != nil || info.Size() == 0 { + t.Fatalf("expected non-empty Judge artifact %s: info=%v err=%v", relativePath, info, err) + } + } +} + // TestPipeline_MustContainPass verifies that a case with matching must_contain // keywords passes the expect check when the mock engine returns the right output. func TestPipeline_MustContainPass(t *testing.T) { diff --git a/e2e/testdata/mock-engine/engine.sh b/e2e/testdata/mock-engine/engine.sh index 3e332b0..0cf0539 100755 --- a/e2e/testdata/mock-engine/engine.sh +++ b/e2e/testdata/mock-engine/engine.sh @@ -18,6 +18,8 @@ # MOCK_CREATE_FILE - if set, create this file in the current working directory # MOCK_FAIL_COUNT - fail the first N invocations, then succeed (requires MOCK_STATE_FILE) # MOCK_STATE_FILE - path to a temp file used as invocation counter for MOCK_FAIL_COUNT +# MOCK_JUDGE_CORRECTION_RETRY - emit an invalid initial Judge response and a valid correction +# MOCK_JUDGE_STATE_FILE - optional file recording each Judge protocol attempt # # Judge prompt auto-detection: # When the prompt contains "Required Response Format" AND "## Criteria", @@ -80,6 +82,23 @@ if [[ -n "${MOCK_CREATE_FILE:-}" ]]; then echo "Generated by mock engine" > "$MOCK_CREATE_FILE" fi +# Handle an agent_judge correction prompt before the initial-prompt detector: +# fallback retries contain the original prompt in their message history. +if [[ "${MOCK_JUDGE_CORRECTION_RETRY:-}" == "1" ]] && echo "$PROMPT" | grep -q "Agent Judge Output Correction"; then + if [[ -n "${MOCK_JUDGE_STATE_FILE:-}" ]]; then + echo "correction" >> "$MOCK_JUDGE_STATE_FILE" + fi + JSON_RESULTS=$(printf '%s' "$PROMPT" | python3 -c ' +import json, re, sys +prompt = sys.stdin.read() +ids = list(dict.fromkeys(re.findall(r"criterion_id\"\s*:\s*\"(criterion-[0-9]+)\"", prompt))) +items = [{"criterion_id": criterion_id, "passed": True, "evidence": ["Mock engine corrected the response contract"], "failures": []} for criterion_id in ids] +print(json.dumps({"results": items})) +') + echo "$JSON_RESULTS" + exit "${MOCK_EXIT_CODE:-0}" +fi + # Handle agent_judge evaluation prompts. # The judge prompt always contains "Required Response Format" and "## Criteria". # We parse the stable criterion IDs and return a valid JSON response so that @@ -97,6 +116,18 @@ for line in sys.stdin: items.append({"criterion_id": criterion_id, "passed": True, "evidence": ["Mock engine: criterion satisfied based on agent output analysis"], "failures": []}) print(json.dumps({"results": items})) ') + if [[ "${MOCK_JUDGE_CORRECTION_RETRY:-}" == "1" ]]; then + if [[ -n "${MOCK_JUDGE_STATE_FILE:-}" ]]; then + echo "initial" >> "$MOCK_JUDGE_STATE_FILE" + fi + JSON_RESULTS=$(printf '%s' "$JSON_RESULTS" | python3 -c ' +import json, sys +payload = json.load(sys.stdin) +for item in payload["results"]: + item.pop("passed", None) +print(json.dumps(payload)) +') + fi echo "$JSON_RESULTS" exit "${MOCK_EXIT_CODE:-0}" fi diff --git a/internal/agent/custom_test.go b/internal/agent/custom_test.go index 5e4c747..1f4b1c8 100644 --- a/internal/agent/custom_test.go +++ b/internal/agent/custom_test.go @@ -32,6 +32,32 @@ func userMessages() []transcript.Message { return []transcript.Message{{Role: transcript.RoleUser, Content: "review the diff"}} } +func TestCustomAgent_RunLocal_PromptPreservesSerializedCorrection(t *testing.T) { + t.Parallel() + rt := newCustomTestRuntime(t) + correctionEnvelope := "original judge request\nprevious invalid response\ncorrection instructions" + ag := customLocalAgent(&config.CustomEngineConfig{ + Transport: "local", + ResponseFormat: "text", + Local: &config.CustomLocalConfig{ + Command: "printf", + Args: []string{"%s", "${prompt}"}, + }, + }) + + result, err := ag.Run(context.Background(), rt, ExecOptions{}, []transcript.Message{{ + Role: transcript.RoleUser, + Content: correctionEnvelope, + Turn: 2, + }}) + if err != nil { + t.Fatalf("Run: %v", err) + } + if result.FinalMessage != correctionEnvelope { + t.Fatalf("custom ${prompt} lost correction context: got %q want %q", result.FinalMessage, correctionEnvelope) + } +} + func TestCustomAgent_RunLocal_StdoutSessionResult(t *testing.T) { t.Parallel() rt := newCustomTestRuntime(t) diff --git a/internal/evaluator/README.md b/internal/evaluator/README.md index af1f36b..0bfe658 100644 --- a/internal/evaluator/README.md +++ b/internal/evaluator/README.md @@ -158,6 +158,13 @@ Artifacts include: - `stdout.txt` — standard output - `*.jsonl` — trace files +For `agent_judge`, artifacts live under `outputs/judge/run/`. The first engine +artifact paths remain unchanged and `raw-response-attempt-1.txt` is always +written. When strict output correction is needed, the directory also contains +`raw-response-attempt-2.txt` and a `retry/` subdirectory for the second engine +invocation. Evaluator preserves this layout on both successful corrections and +final Judge errors. + --- ## Orchestration Flow diff --git a/internal/evaluator/evaluator_test.go b/internal/evaluator/evaluator_test.go index a0b682f..7541377 100644 --- a/internal/evaluator/evaluator_test.go +++ b/internal/evaluator/evaluator_test.go @@ -1802,12 +1802,12 @@ func TestRetryBackoffDelay(t *testing.T) { } } -func TestExecuteCase_JudgeErrorDownloadsJudgeArtifacts(t *testing.T) { +func TestExecuteCase_JudgeErrorDownloadsJudgeArtifacts(t *testing.T) { //nolint:cyclop,funlen,gocyclo // Error-path test verifies both retry attempts and artifact recovery. t.Parallel() rt := &mockRuntime{workspace: t.TempDir()} ag := &mockAgent{name: "test"} - ag.runFunc = func(_ context.Context, rt runtime.Runtime, _ agent.ExecOptions, _ []transcript.Message) (*agent.SessionResult, error) { + ag.runFunc = func(_ context.Context, rt runtime.Runtime, opts agent.ExecOptions, messages []transcript.Message) (*agent.SessionResult, error) { callNum := ag.runCall.Load() switch callNum { case 1: @@ -1828,6 +1828,27 @@ func TestExecuteCase_JudgeErrorDownloadsJudgeArtifacts(t *testing.T) { }, } return session, errors.New("API rate limit exceeded") + case 3: + if filepath.Base(opts.ArtifactDir) != "retry" { + t.Fatalf("correction ArtifactDir = %q, want retry subdirectory", opts.ArtifactDir) + } + if len(messages) != 1 || messages[0].Role != transcript.RoleUser || + !strings.Contains(messages[0].Content, "Agent Judge Output Correction") || + !strings.Contains(messages[0].Content, `API Error: 400 rate limit`) { + t.Fatalf("expected serialized correction fallback, got %#v", messages) + } + if err := os.WriteFile(filepath.Join(rt.Workspace(), "judge-retry-stdout.json"), []byte(`{"error":"invalid correction"}`), 0o600); err != nil { + t.Fatalf("write retry judge artifact: %v", err) + } + return &agent.SessionResult{ + FinalMessage: `{"results":null}`, + DurationMs: 700, + InputTokens: 40, + OutputTokens: 10, + Artifacts: &agent.SessionArtifacts{ + GeneratedFiles: []string{"judge-retry-stdout.json"}, + }, + }, nil default: t.Fatalf("unexpected run call %d", callNum) return nil, nil @@ -1859,12 +1880,182 @@ func TestExecuteCase_JudgeErrorDownloadsJudgeArtifacts(t *testing.T) { if result.JudgeSession == nil { t.Fatal("expected failed judge session to be preserved") } - if result.JudgeSession.DurationMs != 2300 || result.JudgeSession.InputTokens != 120 || result.JudgeSession.OutputTokens != 30 { + if result.JudgeSession.DurationMs != 3000 || result.JudgeSession.InputTokens != 160 || result.JudgeSession.OutputTokens != 40 { t.Fatalf("unexpected failed judge metrics: %#v", result.JudgeSession) } - if rt.downloadFileCall.Load() == 0 { - t.Fatal("expected judge artifacts to be downloaded on judge error") + if rt.downloadFileCall.Load() != 2 { + t.Fatalf("expected both judge engine artifacts to be downloaded on judge error, got %d", rt.downloadFileCall.Load()) + } + if result.JudgeSession.Artifacts == nil || len(result.JudgeSession.Artifacts.GeneratedFiles) != 4 { + t.Fatalf("expected both engine and raw response artifacts, got %#v", result.JudgeSession.Artifacts) + } +} + +func TestExecuteCase_JudgeCorrectionPreservesAttemptArtifactsOnSuccess(t *testing.T) { //nolint:cyclop,gocyclo // End-to-end evaluator assertion covers the full artifact layout. + t.Parallel() + + outputDir := t.TempDir() + rt := &mockRuntime{workspace: t.TempDir()} + ag := &mockAgent{name: "test"} + ag.runFunc = func(_ context.Context, _ runtime.Runtime, opts agent.ExecOptions, messages []transcript.Message) (*agent.SessionResult, error) { + switch ag.runCall.Load() { + case 1: + return &agent.SessionResult{FinalMessage: "main result"}, nil + case 2: + path := filepath.Join(opts.ArtifactDir, "stdout.json") + if err := os.WriteFile(path, []byte(`{"attempt":1}`), 0o600); err != nil { + t.Fatalf("write first judge artifact: %v", err) + } + return &agent.SessionResult{ + FinalMessage: `{"results":null}`, + Artifacts: &agent.SessionArtifacts{GeneratedFiles: []string{path}}, + }, nil + case 3: + if len(messages) != 1 || messages[0].Role != transcript.RoleUser || filepath.Base(opts.ArtifactDir) != "retry" { + t.Fatalf("unexpected correction call: dir=%q messages=%#v", opts.ArtifactDir, messages) + } + if err := os.MkdirAll(opts.ArtifactDir, 0o755); err != nil { + t.Fatalf("create retry artifact dir: %v", err) + } + path := filepath.Join(opts.ArtifactDir, "stdout.json") + if err := os.WriteFile(path, []byte(`{"attempt":2}`), 0o600); err != nil { + t.Fatalf("write retry judge artifact: %v", err) + } + return &agent.SessionResult{ + FinalMessage: `{"results":[{"criterion_id":"criterion-1","passed":true,"evidence":["corrected"],"failures":[]}]}`, + Artifacts: &agent.SessionArtifacts{GeneratedFiles: []string{path}}, + }, nil + default: + t.Fatalf("unexpected run call %d", ag.runCall.Load()) + return nil, nil + } + } + + e := newTestEvaluator(EvalOptions{Agent: ag, OutputDir: outputDir}) + caseCfg := &config.CaseConfig{ + ID: "case-judge-correction", + Title: "Judge Correction", + Input: config.Input{Prompt: "hello"}, + Judge: config.JudgeConfig{ + Type: "agent_judge", + Model: "test-model", + Criteria: []string{"criterion"}, + }, + } + + result := e.executeCase(context.Background(), caseCfg, "with_skill", rt, nil) + if result.Status != judge.StatusPass { + t.Fatalf("expected PASS status, got %s: %v", result.Status, result.Error) + } + if ag.runCall.Load() != 3 { + t.Fatalf("expected main run plus two judge attempts, got %d", ag.runCall.Load()) + } + judgeDir := filepath.Join(outputDir, caseCfg.ID, "with_skill", "outputs", "judge", "run") + for _, relativePath := range []string{ + "stdout.json", + "raw-response-attempt-1.txt", + filepath.Join("retry", "stdout.json"), + "raw-response-attempt-2.txt", + } { + if _, err := os.Stat(filepath.Join(judgeDir, relativePath)); err != nil { + t.Fatalf("expected judge artifact %s: %v", relativePath, err) + } + } + if result.JudgeSession == nil || result.JudgeSession.Artifacts == nil || len(result.JudgeSession.Artifacts.GeneratedFiles) != 4 { + t.Fatalf("expected aggregated judge artifacts, got %#v", result.JudgeSession) + } +} + +func TestExecuteCase_CustomJudgeCorrectionSnapshotsFrameworkFiles(t *testing.T) { //nolint:funlen // Integration coverage intentionally exercises the real CustomAgent and evaluator artifact pipeline. + outputDir := t.TempDir() + scriptPath := filepath.Join(t.TempDir(), "custom-judge.sh") + script := `#!/bin/sh +set -eu +output_file=$2 +workspace=$3 +counter_file="$workspace/custom-judge-calls" +call_count=0 +if [ -f "$counter_file" ]; then + call_count=$(cat "$counter_file") +fi +call_count=$((call_count + 1)) +printf '%s' "$call_count" > "$counter_file" +mkdir -p "$(dirname "$output_file")" +case "$call_count" in + 1) result='{"exit_code":0,"final_message":"main result"}' ;; + 2) result='{"exit_code":0,"final_message":"not-json"}' ;; + 3) result='{"exit_code":0,"final_message":"{\"results\":[{\"criterion_id\":\"criterion-1\",\"passed\":true,\"evidence\":[\"corrected\"],\"failures\":[]}] }"}' ;; + *) exit 2 ;; +esac +printf '%s\n' "$result" > "$output_file" +` + if err := os.WriteFile(scriptPath, []byte(script), 0o600); err != nil { + t.Fatalf("write custom engine script: %v", err) + } + + rt := &runtime.NoneRuntime{} + if err := rt.Create(context.Background()); err != nil { + t.Fatalf("create none runtime: %v", err) + } + t.Cleanup(func() { _ = rt.Close() }) + customAgent := agent.NewCustomAgent(agent.Config{ + Name: "custom-judge", + Custom: &config.CustomEngineConfig{ + Transport: "local", + Local: &config.CustomLocalConfig{ + Command: "sh", + Args: []string{scriptPath, "${input_file}", "${output_file}", "${workspace}"}, + OutputFile: "${output_file}", + }, + }, + }) + e := newTestEvaluator(EvalOptions{Agent: customAgent, OutputDir: outputDir}) + passThreshold := 1.0 + caseCfg := &config.CaseConfig{ + ID: "custom-judge-correction", + Title: "Custom Judge Correction", + Input: config.Input{Prompt: "main prompt"}, + Judge: config.JudgeConfig{ + Type: "agent_judge", + Criteria: []string{"configured criterion"}, + PassThreshold: &passThreshold, + }, + } + + result := e.executeCase(context.Background(), caseCfg, "with_skill", rt, nil) + if result.Status != judge.StatusPass { + t.Fatalf("expected PASS status, got %s: %v", result.Status, result.Error) + } + if result.JudgeSession == nil || result.JudgeSession.Turns != 2 { + t.Fatalf("two independent Judge runs must report two turns, got %#v", result.JudgeSession) + } + + judgeDir := filepath.Join(outputDir, caseCfg.ID, "with_skill", "outputs", "judge", "run") + firstInput := readTestFile(t, filepath.Join(judgeDir, "messages.json")) + retryInput := readTestFile(t, filepath.Join(judgeDir, "retry", "messages.json")) + firstOutput := readTestFile(t, filepath.Join(judgeDir, "session-result.json")) + retryOutput := readTestFile(t, filepath.Join(judgeDir, "retry", "session-result.json")) + if !strings.Contains(firstInput, "configured criterion") || strings.Contains(firstInput, "Agent Judge Output Correction") { + t.Fatalf("unexpected first Judge input snapshot: %s", firstInput) + } + if !strings.Contains(retryInput, "Agent Judge Output Correction") || !strings.Contains(retryInput, "not-json") { + t.Fatalf("retry input snapshot lost correction context: %s", retryInput) + } + if !strings.Contains(firstOutput, `"final_message":"not-json"`) { + t.Fatalf("first output snapshot was overwritten: %s", firstOutput) + } + if !strings.Contains(retryOutput, `criterion-1`) || strings.Contains(retryOutput, `"final_message":"not-json"`) { + t.Fatalf("unexpected retry output snapshot: %s", retryOutput) + } +} + +func readTestFile(t *testing.T, path string) string { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", path, err) } + return string(data) } func TestExecuteCase_CaseLevelJudge(t *testing.T) { diff --git a/internal/judge/README.md b/internal/judge/README.md index 67d1a99..6867a62 100644 --- a/internal/judge/README.md +++ b/internal/judge/README.md @@ -198,13 +198,30 @@ Rule assertions: - `pass_threshold` (default 0.7): `pass_rate >= threshold` → PASS - Provides the internal helper `buildJudgePrompt()` to construct the evaluation prompt - Accepts one strict JSON object, optionally wrapped in one complete JSON code fence; unknown fields and trailing content are rejected +- Retries exactly once when strict decoding or semantic validation fails. The retry shares the original timeout budget and only asks the Judge agent to correct its output contract. +- Resumes the Judge session when the agent exposes `SessionResumer` and a session ID; otherwise it starts an independent first turn and serializes the original prompt, raw response, and correction instructions into one user message so single-instruction adapters preserve the complete context. +- Does not retry a valid FAIL judgment, explicit context cancellation, context materialization failure, or an agent error without a recoverable response. +- Persists every raw Judge response without rewriting it. The first engine artifacts remain at their existing paths and retry engine artifacts use a `retry/` subdirectory: + +```text +outputs/judge/run/ +├── stdout.json (or other first-attempt engine artifacts) +├── messages.json / session-result.json # snapshotted Custom Engine framework files, when used +├── raw-response-attempt-1.txt +├── raw-response-attempt-2.txt # only when correction runs +└── retry/ + ├── stdout.json (or other retry engine artifacts) + └── messages.json / session-result.json # retry snapshots, when used +``` + +Runtime-backed attempt artifacts are snapshotted before another call can overwrite their workspace paths. The final Judge session represents the last attempt, while duration and usage metrics include the complete correction flow. Invalid output after the single correction remains an ERROR and preserves both attempts for diagnosis. ## Security - Workspace file checks (`files_exist`, `files_not_exist`) and `golden_file` all go through `safePath()`. The first two are relative to the workspace; `golden_file` is relative to the skill root. Both prevent `../` path-traversal attacks. - File-existence checks share a single helper, `fileExistsInWorkspace()`, ensuring that the `expect` layer and the `rule_based` layer use the same path-safety validation and detection logic. -- Invalid agent judge responses remain errors and preserve the judge session for artifact collection. +- Invalid agent judge responses remain errors after the bounded correction attempt and preserve the aggregated judge session for artifact collection. ## Testing diff --git a/internal/judge/agent_judge.go b/internal/judge/agent_judge.go index fca3c63..b92c48e 100644 --- a/internal/judge/agent_judge.go +++ b/internal/judge/agent_judge.go @@ -6,6 +6,10 @@ import ( "errors" "fmt" "io" + "os" + "path/filepath" + "reflect" + "slices" "strings" "time" @@ -16,8 +20,21 @@ import ( "github.com/alibaba/skill-up/pkg/transcript" ) -// DefaultPassThreshold is the default minimum pass rate for agent_judge. -const DefaultPassThreshold = 0.7 +const ( + // DefaultPassThreshold is the default minimum pass rate for agent_judge. + DefaultPassThreshold = 0.7 + + agentJudgeRawResponseAttempt1 = "raw-response-attempt-1.txt" + agentJudgeRawResponseAttempt2 = "raw-response-attempt-2.txt" + agentJudgeRetryArtifactDir = "retry" +) + +type agentJudgeCorrectionMode uint8 + +const ( + agentJudgeCorrectionIndependent agentJudgeCorrectionMode = iota + agentJudgeCorrectionResumed +) // --------------------------------------------------------------------------- // Data types for agent_judge JSON parsing @@ -138,44 +155,315 @@ func (j *AgentJudge) Evaluate(ctx context.Context, in Input) (*Result, error) { 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 - // instant Run returns, before parent's timer has any chance to fire on - // its own; this is how we distinguish a judge-level deadline from a - // parent (case-level) one and decide whether to annotate. Reading - // parentCtx.Err() later would race against the parent timer in the - // caseTimeout ≈ judgeTimeout boundary. - sessionResult, err := j.Agent.Run(ctx, j.Runtime, agent.ExecOptions{ArtifactDir: in.ArtifactDir}, messages) - parentExpired := parentCtx.Err() != nil - if err != nil { - annotated := err - if !parentExpired { - annotated = j.annotateTimeoutError(ctx, err) - } - if !canRecoverAgentJudgeResult(err, sessionResult) { - return nil, &SessionResultError{ - Err: fmt.Errorf("agent_judge agent call failed: %w", annotated), - Session: sessionResult, - } + // Snapshot parentCtx.Err() immediately after each call so a parent timer + // firing later is not mislabeled as judge.timeout_seconds. + firstSession, runErr := j.Agent.Run(ctx, j.Runtime, agent.ExecOptions{ArtifactDir: in.ArtifactDir}, messages) + if firstSession == nil && runErr == nil { + runErr = errors.New("agent returned no session result") + } + parentErr := parentCtx.Err() + persistAgentJudgeAttemptArtifacts(ctx, j.Runtime, in.ArtifactDir, in.ArtifactDir, agentJudgeRawResponseAttempt1, firstSession) + if callErr := j.agentCallError(ctx, runErr, firstSession, parentErr, "agent call"); callErr != nil { + return nil, &SessionResultError{Err: callErr, Session: firstSession} + } + + criterionResults, validationErr := parseAgentJudgeResults(j.Criteria, firstSession.FinalMessage) + if validationErr == nil { + return j.buildResult(in, firstSession, materialized, prompt, criterionResults), nil + } + if ctx.Err() != nil { + return nil, &SessionResultError{ + Err: fmt.Errorf( + "agent_judge output validation failed and correction retry could not start: %w", + errors.Join(validationErr, ctx.Err()), + ), + Session: firstSession, } - logging.WarnContextf(ctx, "agent_judge recovering judge output despite agent error: %v (judge.timeout_seconds=%d, parent_ctx_expired=%t)", err, j.TimeoutSeconds, parentExpired) } + logging.WarnContextf(ctx, "agent_judge output failed validation; retrying once with correction guidance: %v", validationErr) - var resp judgeResponse - if err := decodeAgentJudgeResponse(sessionResult.FinalMessage, &resp); err != nil { + correctionPrompt := buildAgentJudgeCorrectionPrompt(j.Criteria, validationErr) + retryArtifactDir := "" + if in.ArtifactDir != "" { + retryArtifactDir = filepath.Join(in.ArtifactDir, agentJudgeRetryArtifactDir) + } + retrySession, correctionMode, retryErr := j.runAgentJudgeCorrection(ctx, firstSession, prompt, correctionPrompt, retryArtifactDir) + if retrySession == nil && retryErr == nil { + retryErr = errors.New("agent returned no session result") + } + parentErr = parentCtx.Err() + persistAgentJudgeAttemptArtifacts(ctx, j.Runtime, retryArtifactDir, in.ArtifactDir, agentJudgeRawResponseAttempt2, retrySession) + aggregateSession := aggregateAgentJudgeSessions(firstSession, retrySession, correctionMode) + if callErr := j.agentCallError(ctx, retryErr, retrySession, parentErr, "correction call"); callErr != nil { return nil, &SessionResultError{ - Err: fmt.Errorf("agent_judge failed to parse agent output: %w", err), - Session: sessionResult, + Err: fmt.Errorf( + "agent_judge correction retry failed after initial validation error %q: %w", + validationErr.Error(), + callErr, + ), + Session: aggregateSession, } } - criterionResults, err := validateAgentJudgeResponse(j.Criteria, resp.Results) - if err != nil { + + criterionResults, correctionErr := parseAgentJudgeResults(j.Criteria, retrySession.FinalMessage) + if correctionErr != nil { return nil, &SessionResultError{ - Err: err, - Session: sessionResult, + Err: fmt.Errorf( + "agent_judge correction retry remained invalid after initial validation error %q: %w", + validationErr.Error(), + correctionErr, + ), + Session: aggregateSession, } } + logging.DebugContextf(ctx, "agent_judge correction retry produced a valid response") + return j.buildResult(in, aggregateSession, materialized, prompt, criterionResults), nil +} + +func (j *AgentJudge) agentCallError(ctx context.Context, err error, sessionResult *agent.SessionResult, parentErr error, callLabel string) error { + if err == nil { + return nil + } + annotated := err + if parentErr == nil { + annotated = j.annotateTimeoutError(ctx, err) + } + if !canRecoverAgentJudgeResult(err, sessionResult) { + return fmt.Errorf("agent_judge %s failed: %w", callLabel, annotated) + } + if callLabel == "agent call" { + logging.WarnContextf( + ctx, + "agent_judge recovering judge output despite agent error: %v (judge.timeout_seconds=%d, parent_ctx_expired=%t)", + err, + j.TimeoutSeconds, + parentErr != nil, + ) + } else { + logging.WarnContextf( + ctx, + "agent_judge recovering output from %s despite agent error: %v (judge.timeout_seconds=%d, parent_ctx_expired=%t)", + callLabel, + err, + j.TimeoutSeconds, + parentErr != nil, + ) + } + return nil +} + +func (j *AgentJudge) runAgentJudgeCorrection( + ctx context.Context, + firstSession *agent.SessionResult, + originalPrompt, + correctionPrompt, + artifactDir string, +) (*agent.SessionResult, agentJudgeCorrectionMode, error) { + opts := agent.ExecOptions{ArtifactDir: artifactDir} + if resumer, ok := j.Agent.(agent.SessionResumer); ok && firstSession != nil && firstSession.SessionID != "" { + sessionResult, err := resumer.RunTurn( + ctx, + j.Runtime, + opts, + transcript.Message{Role: transcript.RoleUser, Content: correctionPrompt, Turn: 2}, + firstSession.SessionID, + ) + return sessionResult, agentJudgeCorrectionResumed, err + } + + invalidResponse := "" + if firstSession != nil { + invalidResponse = firstSession.FinalMessage + } + fallbackPrompt := buildAgentJudgeFallbackCorrectionPrompt(originalPrompt, invalidResponse, correctionPrompt) + sessionResult, err := j.Agent.Run(ctx, j.Runtime, opts, []transcript.Message{{ + Role: transcript.RoleUser, + Content: fallbackPrompt, + Turn: 1, + }}) + return sessionResult, agentJudgeCorrectionIndependent, err +} + +func parseAgentJudgeResults(criteria []string, output string) ([]CriterionResult, error) { + var resp judgeResponse + if err := decodeAgentJudgeResponse(output, &resp); err != nil { + return nil, fmt.Errorf("agent_judge failed to parse agent output: %w", err) + } + return validateAgentJudgeResponse(criteria, resp.Results) +} + +func persistAgentJudgeAttemptArtifacts( + ctx context.Context, + rt runtime.Runtime, + attemptArtifactDir, + rawArtifactDir, + rawFileName string, + sessionResult *agent.SessionResult, +) { + snapshotAgentJudgeAttemptArtifacts(ctx, rt, attemptArtifactDir, sessionResult) + persistAgentJudgeRawResponse(ctx, rawArtifactDir, rawFileName, sessionResult) +} - return j.buildResult(in, sessionResult, materialized, prompt, criterionResults), nil +func persistAgentJudgeRawResponse(ctx context.Context, artifactDir, fileName string, sessionResult *agent.SessionResult) { + if artifactDir == "" || sessionResult == nil { + return + } + if err := os.MkdirAll(artifactDir, 0o755); err != nil { + logging.WarnContextf(ctx, "agent_judge failed to create raw response artifact directory %s: %v", artifactDir, err) + return + } + path := filepath.Join(artifactDir, fileName) + if err := os.WriteFile(path, []byte(sessionResult.FinalMessage), 0o600); err != nil { + logging.WarnContextf(ctx, "agent_judge failed to persist raw response artifact %s: %v", path, err) + return + } + if sessionResult.Artifacts == nil { + sessionResult.Artifacts = &agent.SessionArtifacts{} + } + sessionResult.Artifacts.GeneratedFiles = appendUniqueString(sessionResult.Artifacts.GeneratedFiles, path) +} + +// snapshotAgentJudgeAttemptArtifacts preserves runtime-backed artifacts before +// a correction attempt can overwrite their workspace paths. Artifacts already +// materialized inside the attempt directory are left untouched. Snapshot +// failures are best-effort: the original path remains available to the +// evaluator and the Judge result is not changed. +func snapshotAgentJudgeAttemptArtifacts( + ctx context.Context, + rt runtime.Runtime, + artifactDir string, + sessionResult *agent.SessionResult, +) { + if artifactDir == "" || sessionResult == nil || sessionResult.Artifacts == nil { + return + } + if err := os.MkdirAll(artifactDir, 0o755); err != nil { + logging.WarnContextf(ctx, "agent_judge failed to create attempt artifact directory %s: %v", artifactDir, err) + return + } + + generatedFiles := sessionResult.Artifacts.GeneratedFiles + for i, sourcePath := range generatedFiles { + if sourcePath == "" || pathWithinDir(sourcePath, artifactDir) { + continue + } + targetPath := filepath.Join(artifactDir, filepath.Base(sourcePath)) + if err := rt.DownloadFile(ctx, sourcePath, targetPath); err != nil { + logging.WarnContextf(ctx, "agent_judge failed to snapshot attempt artifact %s to %s: %v", sourcePath, targetPath, err) + continue + } + generatedFiles[i] = targetPath + } + sessionResult.Artifacts.GeneratedFiles = appendUniqueStrings(nil, generatedFiles) +} + +func pathWithinDir(path, dir string) bool { + cleanPath := filepath.Clean(path) + cleanDir := filepath.Clean(dir) + if !filepath.IsAbs(cleanPath) || !filepath.IsAbs(cleanDir) { + return false + } + rel, err := filepath.Rel(cleanDir, cleanPath) + return err == nil && rel != "." && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) +} + +func aggregateAgentJudgeSessions(first, second *agent.SessionResult, correctionMode agentJudgeCorrectionMode) *agent.SessionResult { + if first == nil { + return second + } + if second == nil { + return first + } + + aggregate := *second + aggregate.DurationMs = first.DurationMs + second.DurationMs + if correctionMode == agentJudgeCorrectionResumed && judgeSessionMetricsAreCumulative(first, second) { + aggregate.InputTokens = max(first.InputTokens, second.InputTokens) + aggregate.OutputTokens = max(first.OutputTokens, second.OutputTokens) + aggregate.Turns = max(first.Turns, second.Turns) + } else { + aggregate.InputTokens = first.InputTokens + second.InputTokens + aggregate.OutputTokens = first.OutputTokens + second.OutputTokens + aggregate.Turns = first.Turns + second.Turns + } + aggregate.Artifacts = mergeAgentJudgeSessionArtifacts(first.Artifacts, second.Artifacts) + if first.PromptDelivery != nil { + aggregate.PromptDelivery = first.PromptDelivery + } + return &aggregate +} + +func judgeSessionMetricsAreCumulative(first, second *agent.SessionResult) bool { + if len(first.Transcript) == 0 || len(second.Transcript) < len(first.Transcript) { + return false + } + for i := range first.Transcript { + if !reflect.DeepEqual(first.Transcript[i], second.Transcript[i]) { + return false + } + } + return true +} + +func mergeAgentJudgeSessionArtifacts(first, second *agent.SessionArtifacts) *agent.SessionArtifacts { + if first == nil { + return second + } + if second == nil { + return first + } + + merged := *second + if merged.WorkspaceDiff == "" { + merged.WorkspaceDiff = first.WorkspaceDiff + } + merged.GeneratedFiles = appendUniqueStrings(first.GeneratedFiles, second.GeneratedFiles) + merged.Files = appendUniqueArtifactFiles(first.Files, second.Files) + switch { + case first.Logs == "": + case merged.Logs == "": + merged.Logs = first.Logs + case first.Logs != merged.Logs: + merged.Logs = first.Logs + "\n" + merged.Logs + } + return &merged +} + +func appendUniqueStrings(groups ...[]string) []string { + seen := make(map[string]struct{}) + var values []string + for _, group := range groups { + for _, value := range group { + if _, ok := seen[value]; ok { + continue + } + seen[value] = struct{}{} + values = append(values, value) + } + } + return values +} + +func appendUniqueString(values []string, value string) []string { + if slices.Contains(values, value) { + return values + } + return append(values, value) +} + +func appendUniqueArtifactFiles(groups ...[]agent.ArtifactFile) []agent.ArtifactFile { + seen := make(map[agent.ArtifactFile]struct{}) + var files []agent.ArtifactFile + for _, group := range groups { + for _, file := range group { + if _, ok := seen[file]; ok { + continue + } + seen[file] = struct{}{} + files = append(files, file) + } + } + return files } func (j *AgentJudge) buildResult(in Input, sessionResult *agent.SessionResult, materialized *MaterializedContext, prompt string, criterionResults []CriterionResult) *Result { @@ -509,6 +797,44 @@ func buildJudgePrompt(_ context.Context, criteria []string, materialized *Materi return sb.String() } +func buildAgentJudgeCorrectionPrompt(criteria []string, validationErr error) string { + encodedError, err := json.Marshal(validationErr.Error()) + if err != nil { + encodedError = []byte(`"agent_judge response validation failed"`) + } + + var sb strings.Builder + sb.WriteString("## Agent Judge Output Correction\n\n") + sb.WriteString("Your previous response failed the program-owned output contract. Correct the response using the same evaluation. ") + sb.WriteString("Do not add commentary, do not wrap the response in a Markdown fence, and do not introduce fields outside the schema.\n\n") + sb.WriteString("Validation error (JSON string): ") + sb.Write(encodedError) + sb.WriteString("\n\n") + sb.WriteString("Allowed root field: results.\n") + sb.WriteString("Required result fields with exact casing: criterion_id, passed, evidence, failures.\n") + sb.WriteString("Use every configured criterion_id exactly once. Evidence must be a non-empty string array. ") + sb.WriteString("Failures must be empty when passed is true and non-empty when passed is false.\n") + appendRequiredResponseFormat(&sb, criteria) + return sb.String() +} + +func buildAgentJudgeFallbackCorrectionPrompt(originalPrompt, invalidResponse, correctionPrompt string) string { + encodedResponse, err := json.Marshal(invalidResponse) + if err != nil { + encodedResponse = []byte(`""`) + } + + var sb strings.Builder + sb.WriteString("This is a serialized correction retry for a previous agent_judge evaluation.\n\n") + sb.WriteString("## Original Judge Request\n\n") + sb.WriteString(originalPrompt) + sb.WriteString("\n\n## Previous Invalid Response (JSON string)\n\n") + sb.Write(encodedResponse) + sb.WriteString("\n\n") + sb.WriteString(correctionPrompt) + return sb.String() +} + func appendJudgeSkillInstructions(sb *strings.Builder, skills []SkillInfo) { if len(skills) == 0 { return diff --git a/internal/judge/agent_judge_test.go b/internal/judge/agent_judge_test.go index 722707f..f932c01 100644 --- a/internal/judge/agent_judge_test.go +++ b/internal/judge/agent_judge_test.go @@ -3,7 +3,10 @@ package judge import ( "bytes" "context" + "encoding/json" "errors" + "os" + "path/filepath" "strings" "sync" "testing" @@ -99,6 +102,9 @@ func TestAgentJudge_AllFail(t *testing.T) { if r.Summary.PassRate != 0 { t.Fatalf("expected pass_rate 0, got %f", r.Summary.PassRate) } + if ag.runCalls != 1 { + t.Fatalf("valid FAIL judgment must not retry, got %d calls", ag.runCalls) + } } // --------------------------------------------------------------------------- @@ -141,7 +147,6 @@ func TestAgentJudge_AgentError(t *testing.T) { func TestAgentJudge_AgentError_PreservesSession(t *testing.T) { session := &agent.SessionResult{ - FinalMessage: "API Error: 400 rate limit", Artifacts: &agent.SessionArtifacts{ GeneratedFiles: []string{"stdout.json"}, }, @@ -160,6 +165,9 @@ func TestAgentJudge_AgentError_PreservesSession(t *testing.T) { if got := SessionResultFromError(err); got != session { t.Fatalf("expected preserved session result, got %#v", got) } + if ag.runCalls != 1 { + t.Fatalf("unrecoverable agent error must not retry, got %d calls", ag.runCalls) + } } func TestAgentJudge_RecoversTimedOutSessionWithValidJSON(t *testing.T) { @@ -891,17 +899,394 @@ func TestAgentJudge_ConfiguredCriteriaRemainAuthoritative(t *testing.T) { } func TestAgentJudge_InvalidContractPreservesSession(t *testing.T) { - session := &agent.SessionResult{ + firstSession := &agent.SessionResult{ FinalMessage: `{"results":[{"criterion_id":"criterion-1","passed":true,"evidence":["ok"],"failures":[],"score":1}]}`, Artifacts: &agent.SessionArtifacts{GeneratedFiles: []string{"stdout.json"}}, } - j := NewAgentJudge(&mockJudgeTestAgent{runResult: session}, &mockJudgeTestRuntime{}, "test-model", []string{"configured"}, nil, 0) + secondSession := &agent.SessionResult{ + FinalMessage: `{"results":[{"criterion_id":"criterion-1","passed":true,"evidence":["still invalid"],"failures":[],"score":2}]}`, + Artifacts: &agent.SessionArtifacts{GeneratedFiles: []string{"retry/stdout.json"}}, + } + ag := &mockJudgeTestAgent{scriptedResults: []*agent.SessionResult{firstSession, secondSession}} + j := NewAgentJudge(ag, &mockJudgeTestRuntime{}, "test-model", []string{"configured"}, nil, 0) _, err := j.Evaluate(context.Background(), Input{FinalMessage: "test"}) if err == nil { t.Fatal("expected strict contract error") } - if got := SessionResultFromError(err); got != session { - t.Fatalf("expected preserved judge session, got %#v", got) + got := SessionResultFromError(err) + if got == nil || got.FinalMessage != secondSession.FinalMessage { + t.Fatalf("expected final correction session, got %#v", got) + } + if ag.runCalls != 2 { + t.Fatalf("expected exactly one correction retry, got %d calls", ag.runCalls) + } + if got.Artifacts == nil || len(got.Artifacts.GeneratedFiles) != 2 { + t.Fatalf("expected merged artifacts, got %#v", got.Artifacts) + } +} + +func TestAgentJudge_FirstValidResponsePersistsSingleRawArtifact(t *testing.T) { + artifactDir := t.TempDir() + output := buildMockAgentOutput([]CriterionResult{testCriterionResult(0, true, "observed marker")}) + ag := &mockJudgeTestAgent{output: output} + j := NewAgentJudge(ag, &mockJudgeTestRuntime{}, "test-model", []string{"configured criterion"}, nil, 0) + + result, err := j.Evaluate(context.Background(), Input{FinalMessage: "test", ArtifactDir: artifactDir}) + assertNoError(t, err) + assertStatus(t, result, StatusPass) + if ag.runCalls != 1 { + t.Fatalf("valid response must not retry, got %d calls", ag.runCalls) + } + assertFileContents(t, filepath.Join(artifactDir, agentJudgeRawResponseAttempt1), output) + if _, err := os.Stat(filepath.Join(artifactDir, agentJudgeRawResponseAttempt2)); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("attempt 2 artifact should not exist, stat error: %v", err) + } +} + +func TestAgentJudge_CorrectionRetryFallbackSucceeds(t *testing.T) { //nolint:cyclop,gocyclo // One test asserts the complete fallback protocol and aggregate result. + artifactDir := t.TempDir() + firstOutput := `{"results":[{"criterion_id":"criterion-1","evidence":["marker"],"failures":[]},{"criterion_id":"criterion-2","passed":false,"evidence":["missing"],"failures":["missing"]}]}` + secondOutput := buildMockAgentOutput([]CriterionResult{ + testCriterionResult(1, false, "second unmet"), + testCriterionResult(0, true, "first observed"), + }) + ag := &mockJudgeTestAgent{scriptedResults: []*agent.SessionResult{ + {FinalMessage: firstOutput, DurationMs: 11, InputTokens: 2, OutputTokens: 3, Turns: 1}, + {FinalMessage: secondOutput, DurationMs: 13, InputTokens: 5, OutputTokens: 7, Turns: 1}, + }} + threshold := 0.5 + j := NewAgentJudge(ag, &mockJudgeTestRuntime{}, "test-model", []string{"configured first", "configured second"}, &threshold, 5) + + result, err := j.Evaluate(context.Background(), Input{FinalMessage: "test", ArtifactDir: artifactDir}) + assertNoError(t, err) + assertStatus(t, result, StatusPass) + if ag.runCalls != 2 { + t.Fatalf("expected one correction retry, got %d calls", ag.runCalls) + } + if len(ag.allMessages) != 2 || len(ag.allMessages[1]) != 1 { + t.Fatalf("expected one serialized fallback user message, got %#v", ag.allMessages) + } + fallback := ag.allMessages[1][0] + if fallback.Role != transcript.RoleUser || fallback.Turn != 1 { + t.Fatalf("unexpected fallback role: %#v", fallback) + } + encodedFirstOutput, err := json.Marshal(firstOutput) + if err != nil { + t.Fatalf("marshal first output: %v", err) + } + if !strings.Contains(fallback.Content, string(encodedFirstOutput)) { + t.Fatalf("fallback message did not preserve raw output: %q", fallback.Content) + } + if !strings.Contains(fallback.Content, "configured first") || + !strings.Contains(fallback.Content, `"criterion_id": "criterion-1"`) || + !strings.Contains(fallback.Content, "is missing passed") || + !strings.Contains(fallback.Content, "Required result fields with exact casing") { + t.Fatalf("serialized fallback is missing correction context: %q", fallback.Content) + } + renderedInstruction := agent.BuildInstructionFromMessages(ag.allMessages[1]) + if renderedInstruction != strings.TrimSpace(fallback.Content) || !strings.Contains(renderedInstruction, string(encodedFirstOutput)) { + t.Fatalf("CLI adapter input lost the invalid response: %q", renderedInstruction) + } + if len(ag.artifactDirs) != 2 || ag.artifactDirs[0] != artifactDir || ag.artifactDirs[1] != filepath.Join(artifactDir, agentJudgeRetryArtifactDir) { + t.Fatalf("unexpected attempt artifact dirs: %#v", ag.artifactDirs) + } + if len(ag.observedDeadlines) != 2 || !ag.deadlineStates[0] || !ag.deadlineStates[1] || !ag.observedDeadlines[0].Equal(ag.observedDeadlines[1]) { + t.Fatalf("attempts must share one deadline: deadlines=%#v states=%#v", ag.observedDeadlines, ag.deadlineStates) + } + if result.AssertionResults[0].Text != "configured first" || result.AssertionResults[1].Text != "configured second" { + t.Fatalf("configured criteria/order were not authoritative: %#v", result.AssertionResults) + } + if result.JudgeSession.DurationMs != 24 || result.JudgeSession.InputTokens != 7 || result.JudgeSession.OutputTokens != 10 || result.JudgeSession.Turns != 2 { + t.Fatalf("unexpected independent session aggregation: %#v", result.JudgeSession) + } + assertFileContents(t, filepath.Join(artifactDir, agentJudgeRawResponseAttempt1), firstOutput) + assertFileContents(t, filepath.Join(artifactDir, agentJudgeRawResponseAttempt2), secondOutput) +} + +func TestAgentJudge_CorrectionRetryUsesSessionResumer(t *testing.T) { + artifactDir := t.TempDir() + firstOutput := `{"results":null}` + secondOutput := buildMockAgentOutput([]CriterionResult{testCriterionResult(0, true, "corrected")}) + base := &mockJudgeTestAgent{scriptedResults: []*agent.SessionResult{{ + FinalMessage: firstOutput, + SessionID: "judge-session-123", + }}} + ag := &resumableJudgeTestAgent{ + mockJudgeTestAgent: base, + turnResult: &agent.SessionResult{FinalMessage: secondOutput, SessionID: "judge-session-123"}, + } + j := NewAgentJudge(ag, &mockJudgeTestRuntime{}, "test-model", []string{"configured"}, nil, 5) + + result, err := j.Evaluate(context.Background(), Input{FinalMessage: "test", ArtifactDir: artifactDir}) + assertNoError(t, err) + assertStatus(t, result, StatusPass) + if base.runCalls != 1 || ag.turnCalls != 1 { + t.Fatalf("expected one Run and one RunTurn, got Run=%d RunTurn=%d", base.runCalls, ag.turnCalls) + } + if ag.turnSessions[0] != "judge-session-123" || ag.turnMessages[0].Role != transcript.RoleUser { + t.Fatalf("unexpected resumed turn: sessions=%#v messages=%#v", ag.turnSessions, ag.turnMessages) + } + if ag.turnDirs[0] != filepath.Join(artifactDir, agentJudgeRetryArtifactDir) { + t.Fatalf("unexpected retry artifact dir: %q", ag.turnDirs[0]) + } + if !base.observedDeadlineOK || !ag.turnHasLimit || !base.observedDeadline.Equal(ag.turnDeadline) { + t.Fatalf("Run and RunTurn must share one deadline: Run=%v RunTurn=%v", base.observedDeadline, ag.turnDeadline) + } + if result.JudgeSession.SessionID != "judge-session-123" || result.JudgeSession.FinalMessage != secondOutput { + t.Fatalf("last session should be authoritative: %#v", result.JudgeSession) + } +} + +func TestAgentJudge_ResumerWithoutSessionIDUsesFallbackHistory(t *testing.T) { + valid := buildMockAgentOutput([]CriterionResult{testCriterionResult(0, true, "corrected")}) + base := &mockJudgeTestAgent{scriptedResults: []*agent.SessionResult{ + {FinalMessage: `{"results":null}`}, + {FinalMessage: valid}, + }} + ag := &resumableJudgeTestAgent{mockJudgeTestAgent: base} + j := NewAgentJudge(ag, &mockJudgeTestRuntime{}, "test-model", []string{"configured"}, nil, 0) + + result, err := j.Evaluate(context.Background(), Input{FinalMessage: "test"}) + assertNoError(t, err) + assertStatus(t, result, StatusPass) + if base.runCalls != 2 || ag.turnCalls != 0 { + t.Fatalf("empty session ID must use Run fallback, got Run=%d RunTurn=%d", base.runCalls, ag.turnCalls) + } + if len(base.allMessages) != 2 || len(base.allMessages[1]) != 1 || + base.allMessages[1][0].Role != transcript.RoleUser || base.allMessages[1][0].Turn != 1 { + t.Fatalf("expected one serialized fallback user message, got %#v", base.allMessages) + } +} + +func TestAgentJudge_CorrectionRetryStopsAfterTwoInvalidResponses(t *testing.T) { + artifactDir := t.TempDir() + firstOutput := `{"results":null}` + secondOutput := `{"results":[{"criterion_id":"CRITERION-1","passed":true,"evidence":["x"],"failures":[]}]}` + ag := &mockJudgeTestAgent{scriptedResults: []*agent.SessionResult{ + {FinalMessage: firstOutput, SessionID: "first", Artifacts: &agent.SessionArtifacts{GeneratedFiles: []string{"stdout.json"}}}, + {FinalMessage: secondOutput, SessionID: "second", Artifacts: &agent.SessionArtifacts{GeneratedFiles: []string{"retry/stdout.json"}}}, + }} + j := NewAgentJudge(ag, &mockJudgeTestRuntime{}, "test-model", []string{"configured"}, nil, 0) + + _, err := j.Evaluate(context.Background(), Input{FinalMessage: "test", ArtifactDir: artifactDir}) + if err == nil { + t.Fatal("expected correction retry failure") + } + if ag.runCalls != 2 { + t.Fatalf("expected exactly two calls, got %d", ag.runCalls) + } + if !strings.Contains(err.Error(), "results is required") || !strings.Contains(err.Error(), "unknown criterion_id") { + t.Fatalf("error should preserve initial and final validation reasons: %v", err) + } + session := SessionResultFromError(err) + if session == nil || session.SessionID != "second" || session.FinalMessage != secondOutput { + t.Fatalf("expected final session to be preserved: %#v", session) + } + if session.Artifacts == nil || len(session.Artifacts.GeneratedFiles) != 4 { + t.Fatalf("expected engine and raw artifacts from both attempts: %#v", session.Artifacts) + } + assertFileContents(t, filepath.Join(artifactDir, agentJudgeRawResponseAttempt1), firstOutput) + assertFileContents(t, filepath.Join(artifactDir, agentJudgeRawResponseAttempt2), secondOutput) +} + +func TestAgentJudge_InvalidResponseClassesTriggerOneCorrection(t *testing.T) { + valid := buildMockAgentOutput([]CriterionResult{testCriterionResult(0, true, "corrected")}) + tests := []struct { + name string + output string + }{ + {name: "malformed JSON", output: `{"results":[`}, + {name: "unknown field", output: `{"results":[{"criterion_id":"criterion-1","passed":true,"evidence":["x"],"failures":[],"score":1}]}`}, + {name: "missing field", output: `{"results":[{"criterion_id":"criterion-1","passed":true,"evidence":["x"]}]}`}, + {name: "duplicate field", output: `{"results":[{"criterion_id":"criterion-1","passed":false,"passed":true,"evidence":["x"],"failures":[]}]}`}, + {name: "case alias", output: `{"results":[{"Criterion_ID":"criterion-1","passed":true,"evidence":["x"],"failures":[]}]}`}, + {name: "criterion mapping", output: `{"results":[{"criterion_id":"criterion-99","passed":true,"evidence":["x"],"failures":[]}]}`}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ag := &mockJudgeTestAgent{scriptedResults: []*agent.SessionResult{ + {FinalMessage: tt.output}, + {FinalMessage: valid}, + }} + j := NewAgentJudge(ag, &mockJudgeTestRuntime{}, "test-model", []string{"configured"}, nil, 0) + result, err := j.Evaluate(context.Background(), Input{FinalMessage: "test"}) + assertNoError(t, err) + assertStatus(t, result, StatusPass) + if ag.runCalls != 2 { + t.Fatalf("expected one correction retry, got %d calls", ag.runCalls) + } + }) + } +} + +func TestAgentJudge_CanceledContextAfterInvalidResponseDoesNotRetry(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + ag := &mockJudgeTestAgent{ + output: `{"results":null}`, + onRun: func(callIndex int) { + if callIndex == 0 { + cancel() + } + }, + } + j := NewAgentJudge(ag, &mockJudgeTestRuntime{}, "test-model", []string{"configured"}, nil, 0) + + _, err := j.Evaluate(ctx, Input{FinalMessage: "test"}) + if err == nil || !errors.Is(err, context.Canceled) { + t.Fatalf("expected canceled validation error, got %v", err) + } + if ag.runCalls != 1 { + t.Fatalf("canceled context must not retry, got %d calls", ag.runCalls) + } +} + +func TestAgentJudge_CorrectionCallErrors(t *testing.T) { + initialInvalid := &agent.SessionResult{FinalMessage: `{"results":null}`} + valid := buildMockAgentOutput([]CriterionResult{testCriterionResult(0, true, "recovered")}) + + t.Run("recoverable output is parsed", func(t *testing.T) { + ag := &mockJudgeTestAgent{ + scriptedResults: []*agent.SessionResult{initialInvalid, {FinalMessage: valid}}, + scriptedErrors: []error{nil, errors.New("engine exited after response")}, + } + j := NewAgentJudge(ag, &mockJudgeTestRuntime{}, "test-model", []string{"configured"}, nil, 0) + result, err := j.Evaluate(context.Background(), Input{FinalMessage: "test"}) + assertNoError(t, err) + assertStatus(t, result, StatusPass) + if ag.runCalls != 2 { + t.Fatalf("expected two calls, got %d", ag.runCalls) + } + }) + + t.Run("empty output is unrecoverable", func(t *testing.T) { + ag := &mockJudgeTestAgent{ + scriptedResults: []*agent.SessionResult{initialInvalid, {FinalMessage: ""}}, + scriptedErrors: []error{nil, errors.New("engine failed")}, + } + j := NewAgentJudge(ag, &mockJudgeTestRuntime{}, "test-model", []string{"configured"}, nil, 0) + _, err := j.Evaluate(context.Background(), Input{FinalMessage: "test"}) + if err == nil || !strings.Contains(err.Error(), "correction call failed") { + t.Fatalf("expected unrecoverable correction error, got %v", err) + } + if ag.runCalls != 2 { + t.Fatalf("expected two calls, got %d", ag.runCalls) + } + }) + + t.Run("explicit cancellation is unrecoverable", func(t *testing.T) { + ag := &mockJudgeTestAgent{ + scriptedResults: []*agent.SessionResult{initialInvalid, {FinalMessage: valid}}, + scriptedErrors: []error{nil, context.Canceled}, + } + j := NewAgentJudge(ag, &mockJudgeTestRuntime{}, "test-model", []string{"configured"}, nil, 0) + _, err := j.Evaluate(context.Background(), Input{FinalMessage: "test"}) + if err == nil || !errors.Is(err, context.Canceled) { + t.Fatalf("expected explicit cancellation error, got %v", err) + } + if ag.runCalls != 2 { + t.Fatalf("expected two calls, got %d", ag.runCalls) + } + }) +} + +func TestAggregateAgentJudgeSessionsMetrics(t *testing.T) { //nolint:cyclop,gocyclo // Subtests cover resumed and independent aggregation modes together. + firstTranscript := transcript.Transcript{{Role: transcript.RoleUser, Content: "original", Turn: 1}} + firstDelivery := &agent.PromptDeliveryMetadata{Mode: "file", PromptBytes: 101} + first := &agent.SessionResult{ + FinalMessage: "invalid", + DurationMs: 10, + Turns: 1, + InputTokens: 20, + OutputTokens: 5, + Transcript: firstTranscript, + PromptDelivery: firstDelivery, + Artifacts: &agent.SessionArtifacts{ + GeneratedFiles: []string{"first", "shared"}, + Files: []agent.ArtifactFile{{Name: "first", Path: "first"}}, + Logs: "first log", + }, + } + + t.Run("cumulative transcript uses final cumulative metrics", func(t *testing.T) { + second := &agent.SessionResult{ + FinalMessage: "valid", + DurationMs: 12, + Turns: 2, + InputTokens: 35, + OutputTokens: 9, + Transcript: append(append(transcript.Transcript(nil), firstTranscript...), + transcript.Message{Role: transcript.RoleAssistant, Content: "valid", Turn: 2}), + Artifacts: &agent.SessionArtifacts{ + GeneratedFiles: []string{"shared", "second"}, + Files: []agent.ArtifactFile{ + {Name: "first", Path: "first"}, + {Name: "second", Path: "second"}, + }, + Logs: "second log", + }, + } + got := aggregateAgentJudgeSessions(first, second, agentJudgeCorrectionResumed) + if got.DurationMs != 22 || got.Turns != 2 || got.InputTokens != 35 || got.OutputTokens != 9 { + t.Fatalf("unexpected cumulative metrics: %#v", got) + } + if got.PromptDelivery != firstDelivery { + t.Fatalf("first prompt delivery should be retained: %#v", got.PromptDelivery) + } + if got.Artifacts == nil || len(got.Artifacts.GeneratedFiles) != 3 || len(got.Artifacts.Files) != 2 || got.Artifacts.Logs != "first log\nsecond log" { + t.Fatalf("unexpected merged artifacts: %#v", got.Artifacts) + } + }) + + t.Run("independent transcript sums metrics", func(t *testing.T) { + second := &agent.SessionResult{ + DurationMs: 12, + Turns: 1, + InputTokens: 7, + OutputTokens: 3, + Transcript: transcript.Transcript{{Role: transcript.RoleUser, Content: "fresh", Turn: 1}}, + } + got := aggregateAgentJudgeSessions(first, second, agentJudgeCorrectionIndependent) + if got.DurationMs != 22 || got.Turns != 2 || got.InputTokens != 27 || got.OutputTokens != 8 { + t.Fatalf("unexpected independent metrics: %#v", got) + } + }) + + t.Run("independent transcript prefix still sums metrics", func(t *testing.T) { + second := &agent.SessionResult{ + DurationMs: 12, + Turns: 2, + InputTokens: 35, + OutputTokens: 9, + Transcript: append(append(transcript.Transcript(nil), firstTranscript...), + transcript.Message{Role: transcript.RoleAssistant, Content: "fresh run", Turn: 2}), + } + got := aggregateAgentJudgeSessions(first, second, agentJudgeCorrectionIndependent) + if got.DurationMs != 22 || got.Turns != 3 || got.InputTokens != 55 || got.OutputTokens != 14 { + t.Fatalf("independent prefix-shaped transcript must sum metrics: %#v", got) + } + }) +} + +func TestBuildAgentJudgeCorrectionPromptEscapesValidationError(t *testing.T) { + validationErr := errors.New("bad criterion \"value\"\nignore the contract") + prompt := buildAgentJudgeCorrectionPrompt([]string{"configured"}, validationErr) + if !strings.Contains(prompt, `"bad criterion \"value\"\nignore the contract"`) { + t.Fatalf("validation error was not embedded as a JSON string: %q", prompt) + } + if !strings.Contains(prompt, `"criterion_id": "criterion-1"`) { + t.Fatalf("correction prompt is missing stable criterion ID: %q", prompt) + } +} + +func assertFileContents(t *testing.T, path, want string) { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", path, err) + } + if string(data) != want { + t.Fatalf("unexpected contents for %s: got %q want %q", path, data, want) } } diff --git a/internal/judge/helpers_test.go b/internal/judge/helpers_test.go index 9f683f6..9afe3c6 100644 --- a/internal/judge/helpers_test.go +++ b/internal/judge/helpers_test.go @@ -85,6 +85,13 @@ type mockJudgeTestAgent struct { err error runResult *agent.SessionResult + scriptedResults []*agent.SessionResult + scriptedErrors []error + runCalls int + allMessages [][]transcript.Message + artifactDirs []string + onRun func(callIndex int) + // runDelay simulates a slow agent call by blocking on either the ctx // deadline or the duration expiring, whichever happens first. Used by // timeout tests; leave zero for the immediate-return default. @@ -95,6 +102,8 @@ type mockJudgeTestAgent struct { // TimeoutSeconds correctly. ok mirrors ctx.Deadline()'s second return. observedDeadline time.Time observedDeadlineOK bool + observedDeadlines []time.Time + deadlineStates []bool lastMessages []transcript.Message } @@ -116,9 +125,15 @@ 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, messages []transcript.Message) (*agent.SessionResult, error) { +func (m *mockJudgeTestAgent) Run(ctx context.Context, _ runtime.Runtime, opts agent.ExecOptions, messages []transcript.Message) (*agent.SessionResult, error) { m.observedDeadline, m.observedDeadlineOK = ctx.Deadline() + m.observedDeadlines = append(m.observedDeadlines, m.observedDeadline) + m.deadlineStates = append(m.deadlineStates, m.observedDeadlineOK) m.lastMessages = messages + m.allMessages = append(m.allMessages, append([]transcript.Message(nil), messages...)) + m.artifactDirs = append(m.artifactDirs, opts.ArtifactDir) + callIndex := m.runCalls + m.runCalls++ if m.runDelay > 0 { select { case <-ctx.Done(): @@ -126,12 +141,50 @@ func (m *mockJudgeTestAgent) Run(ctx context.Context, _ runtime.Runtime, _ agent case <-time.After(m.runDelay): } } + if m.onRun != nil { + m.onRun(callIndex) + } + if callIndex < len(m.scriptedResults) { + var err error + if callIndex < len(m.scriptedErrors) { + err = m.scriptedErrors[callIndex] + } + return m.scriptedResults[callIndex], err + } if m.runResult != nil { return m.runResult, m.err } return &agent.SessionResult{FinalMessage: m.output}, m.err } +type resumableJudgeTestAgent struct { + *mockJudgeTestAgent + + turnResult *agent.SessionResult + turnErr error + turnCalls int + turnMessages []transcript.Message + turnSessions []string + turnDirs []string + turnDeadline time.Time + turnHasLimit bool +} + +func (m *resumableJudgeTestAgent) RunTurn( + ctx context.Context, + _ runtime.Runtime, + opts agent.ExecOptions, + message transcript.Message, + sessionID string, +) (*agent.SessionResult, error) { + m.turnDeadline, m.turnHasLimit = ctx.Deadline() + m.turnCalls++ + m.turnMessages = append(m.turnMessages, message) + m.turnSessions = append(m.turnSessions, sessionID) + m.turnDirs = append(m.turnDirs, opts.ArtifactDir) + return m.turnResult, m.turnErr +} + // mockJudgeTestRuntime is a minimal Runtime for testing. type mockJudgeTestRuntime struct{}