From 7c9e1b1d88f971e6c1c19da5cc26c1886be8e346 Mon Sep 17 00:00:00 2001 From: Caleb Whitmore <119163801+DancingCircles@users.noreply.github.com> Date: Tue, 18 Aug 2026 20:23:58 +0800 Subject: [PATCH 1/3] =?UTF-8?q?fix(judge):=20=E5=A2=9E=E5=8A=A0=E6=9C=89?= =?UTF-8?q?=E7=95=8C=E7=BA=A0=E9=94=99=E9=87=8D=E8=AF=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 严格协议失败后仅执行一次纠错并共享超时预算 - 保存两次原始响应并隔离重试引擎产物 - 聚合 Judge 会话指标并补齐单元测试与 E2E --- CHANGELOG.md | 5 + e2e/agent_test.go | 8 + e2e/pipeline_test.go | 111 ++++++++ e2e/testdata/mock-engine/engine.sh | 31 +++ internal/evaluator/README.md | 7 + internal/evaluator/evaluator_test.go | 107 +++++++- internal/judge/README.md | 17 +- internal/judge/agent_judge.go | 301 +++++++++++++++++++--- internal/judge/agent_judge_test.go | 369 ++++++++++++++++++++++++++- internal/judge/helpers_test.go | 55 +++- 10 files changed, 969 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 092c8af9..dbf036e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,11 @@ 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 + 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 6c596553..825fde9e 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 d1e7e44e..f207a33f 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 3e332b06..0cf05392 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/evaluator/README.md b/internal/evaluator/README.md index af1f36ba..0bfe6588 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 a0b682f8..752be6b1 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,25 @@ 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) != 3 || !strings.Contains(messages[2].Content, "Agent Judge Output Correction") { + t.Fatalf("expected correction fallback history, 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,11 +1878,89 @@ 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 // 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) != 3 || 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) } } diff --git a/internal/judge/README.md b/internal/judge/README.md index 67d1a997..22a3202a 100644 --- a/internal/judge/README.md +++ b/internal/judge/README.md @@ -198,13 +198,28 @@ 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 replays the original prompt, raw response, and correction prompt as a three-message history. +- 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) +├── raw-response-attempt-1.txt +├── raw-response-attempt-2.txt # only when correction runs +└── retry/ + └── stdout.json (or other retry engine artifacts) +``` + +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 fca3c63f..c8d59da8 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,14 @@ 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" +) // --------------------------------------------------------------------------- // Data types for agent_judge JSON parsing @@ -138,44 +148,254 @@ 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() + persistAgentJudgeRawResponse(ctx, 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, retryErr := j.runAgentJudgeCorrection(ctx, firstSession, messages, correctionPrompt, retryArtifactDir) + if retrySession == nil && retryErr == nil { + retryErr = errors.New("agent returned no session result") + } + parentErr = parentCtx.Err() + persistAgentJudgeRawResponse(ctx, in.ArtifactDir, agentJudgeRawResponseAttempt2, retrySession) + aggregateSession := aggregateAgentJudgeSessions(firstSession, retrySession) + 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, + originalMessages []transcript.Message, + correctionPrompt, + artifactDir string, +) (*agent.SessionResult, error) { + opts := agent.ExecOptions{ArtifactDir: artifactDir} + if resumer, ok := j.Agent.(agent.SessionResumer); ok && firstSession != nil && firstSession.SessionID != "" { + return resumer.RunTurn( + ctx, + j.Runtime, + opts, + transcript.Message{Role: transcript.RoleUser, Content: correctionPrompt, Turn: 2}, + firstSession.SessionID, + ) + } + + messages := make([]transcript.Message, 0, len(originalMessages)+2) + messages = append(messages, originalMessages...) + if firstSession != nil { + messages = append(messages, transcript.Message{Role: transcript.RoleAssistant, Content: firstSession.FinalMessage, Turn: 1}) + } + messages = append(messages, transcript.Message{Role: transcript.RoleUser, Content: correctionPrompt, Turn: 2}) + return j.Agent.Run(ctx, j.Runtime, opts, messages) +} + +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 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) +} + +func aggregateAgentJudgeSessions(first, second *agent.SessionResult) *agent.SessionResult { + if first == nil { + return second + } + if second == nil { + return first + } + + aggregate := *second + aggregate.DurationMs = first.DurationMs + second.DurationMs + if 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 + } - return j.buildResult(in, sessionResult, materialized, prompt, criterionResults), nil + 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 +729,27 @@ 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 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 722707fe..912c1b6e 100644 --- a/internal/judge/agent_judge_test.go +++ b/internal/judge/agent_judge_test.go @@ -4,6 +4,8 @@ import ( "bytes" "context" "errors" + "os" + "path/filepath" "strings" "sync" "testing" @@ -99,6 +101,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 +146,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 +164,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 +898,369 @@ 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]) != 3 { + t.Fatalf("expected original/assistant/correction fallback history, got %#v", ag.allMessages) + } + fallback := ag.allMessages[1] + if fallback[0].Role != transcript.RoleUser || fallback[1].Role != transcript.RoleAssistant || fallback[2].Role != transcript.RoleUser { + t.Fatalf("unexpected fallback roles: %#v", fallback) + } + if fallback[1].Content != firstOutput { + t.Fatalf("fallback assistant message did not preserve raw output: %q", fallback[1].Content) + } + if !strings.Contains(fallback[2].Content, `"criterion_id": "criterion-1"`) || + !strings.Contains(fallback[2].Content, "is missing passed") || + !strings.Contains(fallback[2].Content, "Required result fields with exact casing") { + t.Fatalf("correction prompt is missing contract guidance: %q", fallback[2].Content) + } + 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]) != 3 { + t.Fatalf("expected three-message fallback history, 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) { + 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) + 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) + if got.DurationMs != 22 || got.Turns != 2 || got.InputTokens != 27 || got.OutputTokens != 8 { + t.Fatalf("unexpected independent 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 9f683f62..9afe3c6e 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{} From 422bdaf7139bfcf7d2807e11768bc8e2463b652f Mon Sep 17 00:00:00 2001 From: Caleb Whitmore <119163801+DancingCircles@users.noreply.github.com> Date: Wed, 19 Aug 2026 11:44:56 +0800 Subject: [PATCH 2/3] fix(judge): preserve correction context across adapters - serialize fallback correction context into one user message - distinguish independent and resumed retry usage aggregation - cover CLI and custom prompt rendering regressions --- internal/agent/custom_test.go | 26 +++++++++++++ internal/evaluator/evaluator_test.go | 10 +++-- internal/judge/README.md | 2 +- internal/judge/agent_judge.go | 53 ++++++++++++++++++++------ internal/judge/agent_judge_test.go | 57 ++++++++++++++++++++-------- 5 files changed, 115 insertions(+), 33 deletions(-) diff --git a/internal/agent/custom_test.go b/internal/agent/custom_test.go index 5e4c747e..1f4b1c86 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/evaluator_test.go b/internal/evaluator/evaluator_test.go index 752be6b1..9becbbab 100644 --- a/internal/evaluator/evaluator_test.go +++ b/internal/evaluator/evaluator_test.go @@ -1832,8 +1832,10 @@ func TestExecuteCase_JudgeErrorDownloadsJudgeArtifacts(t *testing.T) { //nolint: if filepath.Base(opts.ArtifactDir) != "retry" { t.Fatalf("correction ArtifactDir = %q, want retry subdirectory", opts.ArtifactDir) } - if len(messages) != 3 || !strings.Contains(messages[2].Content, "Agent Judge Output Correction") { - t.Fatalf("expected correction fallback history, got %#v", messages) + 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) @@ -1889,7 +1891,7 @@ func TestExecuteCase_JudgeErrorDownloadsJudgeArtifacts(t *testing.T) { //nolint: } } -func TestExecuteCase_JudgeCorrectionPreservesAttemptArtifactsOnSuccess(t *testing.T) { //nolint:cyclop // End-to-end evaluator assertion covers the full artifact layout. +func TestExecuteCase_JudgeCorrectionPreservesAttemptArtifactsOnSuccess(t *testing.T) { //nolint:cyclop,gocyclo // End-to-end evaluator assertion covers the full artifact layout. t.Parallel() outputDir := t.TempDir() @@ -1909,7 +1911,7 @@ func TestExecuteCase_JudgeCorrectionPreservesAttemptArtifactsOnSuccess(t *testin Artifacts: &agent.SessionArtifacts{GeneratedFiles: []string{path}}, }, nil case 3: - if len(messages) != 3 || filepath.Base(opts.ArtifactDir) != "retry" { + 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 { diff --git a/internal/judge/README.md b/internal/judge/README.md index 22a3202a..cfe5a060 100644 --- a/internal/judge/README.md +++ b/internal/judge/README.md @@ -199,7 +199,7 @@ Rule assertions: - 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 replays the original prompt, raw response, and correction prompt as a three-message history. +- Resumes the Judge session when the agent exposes `SessionResumer` and a session ID; otherwise it 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: diff --git a/internal/judge/agent_judge.go b/internal/judge/agent_judge.go index c8d59da8..8c6fb5d7 100644 --- a/internal/judge/agent_judge.go +++ b/internal/judge/agent_judge.go @@ -29,6 +29,13 @@ const ( agentJudgeRetryArtifactDir = "retry" ) +type agentJudgeCorrectionMode uint8 + +const ( + agentJudgeCorrectionIndependent agentJudgeCorrectionMode = iota + agentJudgeCorrectionResumed +) + // --------------------------------------------------------------------------- // Data types for agent_judge JSON parsing // --------------------------------------------------------------------------- @@ -180,13 +187,13 @@ func (j *AgentJudge) Evaluate(ctx context.Context, in Input) (*Result, error) { if in.ArtifactDir != "" { retryArtifactDir = filepath.Join(in.ArtifactDir, agentJudgeRetryArtifactDir) } - retrySession, retryErr := j.runAgentJudgeCorrection(ctx, firstSession, messages, correctionPrompt, retryArtifactDir) + 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() persistAgentJudgeRawResponse(ctx, in.ArtifactDir, agentJudgeRawResponseAttempt2, retrySession) - aggregateSession := aggregateAgentJudgeSessions(firstSession, retrySession) + aggregateSession := aggregateAgentJudgeSessions(firstSession, retrySession, correctionMode) if callErr := j.agentCallError(ctx, retryErr, retrySession, parentErr, "correction call"); callErr != nil { return nil, &SessionResultError{ Err: fmt.Errorf( @@ -248,28 +255,33 @@ func (j *AgentJudge) agentCallError(ctx context.Context, err error, sessionResul func (j *AgentJudge) runAgentJudgeCorrection( ctx context.Context, firstSession *agent.SessionResult, - originalMessages []transcript.Message, + originalPrompt, correctionPrompt, artifactDir string, -) (*agent.SessionResult, error) { +) (*agent.SessionResult, agentJudgeCorrectionMode, error) { opts := agent.ExecOptions{ArtifactDir: artifactDir} if resumer, ok := j.Agent.(agent.SessionResumer); ok && firstSession != nil && firstSession.SessionID != "" { - return resumer.RunTurn( + sessionResult, err := resumer.RunTurn( ctx, j.Runtime, opts, transcript.Message{Role: transcript.RoleUser, Content: correctionPrompt, Turn: 2}, firstSession.SessionID, ) + return sessionResult, agentJudgeCorrectionResumed, err } - messages := make([]transcript.Message, 0, len(originalMessages)+2) - messages = append(messages, originalMessages...) + invalidResponse := "" if firstSession != nil { - messages = append(messages, transcript.Message{Role: transcript.RoleAssistant, Content: firstSession.FinalMessage, Turn: 1}) + invalidResponse = firstSession.FinalMessage } - messages = append(messages, transcript.Message{Role: transcript.RoleUser, Content: correctionPrompt, Turn: 2}) - return j.Agent.Run(ctx, j.Runtime, opts, messages) + fallbackPrompt := buildAgentJudgeFallbackCorrectionPrompt(originalPrompt, invalidResponse, correctionPrompt) + sessionResult, err := j.Agent.Run(ctx, j.Runtime, opts, []transcript.Message{{ + Role: transcript.RoleUser, + Content: fallbackPrompt, + Turn: 2, + }}) + return sessionResult, agentJudgeCorrectionIndependent, err } func parseAgentJudgeResults(criteria []string, output string) ([]CriterionResult, error) { @@ -299,7 +311,7 @@ func persistAgentJudgeRawResponse(ctx context.Context, artifactDir, fileName str sessionResult.Artifacts.GeneratedFiles = appendUniqueString(sessionResult.Artifacts.GeneratedFiles, path) } -func aggregateAgentJudgeSessions(first, second *agent.SessionResult) *agent.SessionResult { +func aggregateAgentJudgeSessions(first, second *agent.SessionResult, correctionMode agentJudgeCorrectionMode) *agent.SessionResult { if first == nil { return second } @@ -309,7 +321,7 @@ func aggregateAgentJudgeSessions(first, second *agent.SessionResult) *agent.Sess aggregate := *second aggregate.DurationMs = first.DurationMs + second.DurationMs - if judgeSessionMetricsAreCumulative(first, second) { + 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) @@ -750,6 +762,23 @@ func buildAgentJudgeCorrectionPrompt(criteria []string, validationErr error) str 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 912c1b6e..aac60e23 100644 --- a/internal/judge/agent_judge_test.go +++ b/internal/judge/agent_judge_test.go @@ -3,6 +3,7 @@ package judge import ( "bytes" "context" + "encoding/json" "errors" "os" "path/filepath" @@ -963,20 +964,29 @@ func TestAgentJudge_CorrectionRetryFallbackSucceeds(t *testing.T) { //nolint:cyc if ag.runCalls != 2 { t.Fatalf("expected one correction retry, got %d calls", ag.runCalls) } - if len(ag.allMessages) != 2 || len(ag.allMessages[1]) != 3 { - t.Fatalf("expected original/assistant/correction fallback history, got %#v", ag.allMessages) + 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] - if fallback[0].Role != transcript.RoleUser || fallback[1].Role != transcript.RoleAssistant || fallback[2].Role != transcript.RoleUser { - t.Fatalf("unexpected fallback roles: %#v", fallback) + fallback := ag.allMessages[1][0] + if fallback.Role != transcript.RoleUser { + t.Fatalf("unexpected fallback role: %#v", fallback) } - if fallback[1].Content != firstOutput { - t.Fatalf("fallback assistant message did not preserve raw output: %q", fallback[1].Content) + 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[2].Content, `"criterion_id": "criterion-1"`) || - !strings.Contains(fallback[2].Content, "is missing passed") || - !strings.Contains(fallback[2].Content, "Required result fields with exact casing") { - t.Fatalf("correction prompt is missing contract guidance: %q", fallback[2].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) @@ -1043,8 +1053,8 @@ func TestAgentJudge_ResumerWithoutSessionIDUsesFallbackHistory(t *testing.T) { 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]) != 3 { - t.Fatalf("expected three-message fallback history, got %#v", base.allMessages) + if len(base.allMessages) != 2 || len(base.allMessages[1]) != 1 || base.allMessages[1][0].Role != transcript.RoleUser { + t.Fatalf("expected one serialized fallback user message, got %#v", base.allMessages) } } @@ -1180,7 +1190,7 @@ func TestAgentJudge_CorrectionCallErrors(t *testing.T) { }) } -func TestAggregateAgentJudgeSessionsMetrics(t *testing.T) { +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{ @@ -1216,7 +1226,7 @@ func TestAggregateAgentJudgeSessionsMetrics(t *testing.T) { Logs: "second log", }, } - got := aggregateAgentJudgeSessions(first, second) + 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) } @@ -1236,11 +1246,26 @@ func TestAggregateAgentJudgeSessionsMetrics(t *testing.T) { OutputTokens: 3, Transcript: transcript.Transcript{{Role: transcript.RoleUser, Content: "fresh", Turn: 1}}, } - got := aggregateAgentJudgeSessions(first, second) + 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) { From aed0cdd419b4b0eeeb0171ab086893cd207ef032 Mon Sep 17 00:00:00 2001 From: Caleb Whitmore <119163801+DancingCircles@users.noreply.github.com> Date: Wed, 19 Aug 2026 15:09:29 +0800 Subject: [PATCH 3/3] fix(judge): isolate independent correction attempts - count fresh correction runs from turn one - snapshot runtime artifacts before retry overwrite - cover CustomAgent artifact isolation through evaluator --- CHANGELOG.md | 6 +- internal/evaluator/evaluator_test.go | 92 ++++++++++++++++++++++++++++ internal/judge/README.md | 8 ++- internal/judge/agent_judge.go | 62 ++++++++++++++++++- internal/judge/agent_judge_test.go | 5 +- 5 files changed, 163 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dbf036e5..5ad0f032 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,8 +14,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `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 - Judge duration and usage metrics cover the complete correction flow without - changing report schemas. + 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/internal/evaluator/evaluator_test.go b/internal/evaluator/evaluator_test.go index 9becbbab..75413776 100644 --- a/internal/evaluator/evaluator_test.go +++ b/internal/evaluator/evaluator_test.go @@ -1966,6 +1966,98 @@ func TestExecuteCase_JudgeCorrectionPreservesAttemptArtifactsOnSuccess(t *testin } } +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) { e := newTestEvaluator(EvalOptions{ Agent: &mockAgent{name: "test", output: "hello world"}, diff --git a/internal/judge/README.md b/internal/judge/README.md index cfe5a060..6867a628 100644 --- a/internal/judge/README.md +++ b/internal/judge/README.md @@ -199,20 +199,22 @@ Rule assertions: - 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 serializes the original prompt, raw response, and correction instructions into one user message so single-instruction adapters preserve the complete context. +- 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) + ├── stdout.json (or other retry engine artifacts) + └── messages.json / session-result.json # retry snapshots, when used ``` -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. +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 diff --git a/internal/judge/agent_judge.go b/internal/judge/agent_judge.go index 8c6fb5d7..b92c48ea 100644 --- a/internal/judge/agent_judge.go +++ b/internal/judge/agent_judge.go @@ -162,7 +162,7 @@ func (j *AgentJudge) Evaluate(ctx context.Context, in Input) (*Result, error) { runErr = errors.New("agent returned no session result") } parentErr := parentCtx.Err() - persistAgentJudgeRawResponse(ctx, in.ArtifactDir, agentJudgeRawResponseAttempt1, firstSession) + 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} } @@ -192,7 +192,7 @@ func (j *AgentJudge) Evaluate(ctx context.Context, in Input) (*Result, error) { retryErr = errors.New("agent returned no session result") } parentErr = parentCtx.Err() - persistAgentJudgeRawResponse(ctx, in.ArtifactDir, agentJudgeRawResponseAttempt2, retrySession) + 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{ @@ -279,7 +279,7 @@ func (j *AgentJudge) runAgentJudgeCorrection( sessionResult, err := j.Agent.Run(ctx, j.Runtime, opts, []transcript.Message{{ Role: transcript.RoleUser, Content: fallbackPrompt, - Turn: 2, + Turn: 1, }}) return sessionResult, agentJudgeCorrectionIndependent, err } @@ -292,6 +292,18 @@ func parseAgentJudgeResults(criteria []string, output string) ([]CriterionResult 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) +} + func persistAgentJudgeRawResponse(ctx context.Context, artifactDir, fileName string, sessionResult *agent.SessionResult) { if artifactDir == "" || sessionResult == nil { return @@ -311,6 +323,50 @@ func persistAgentJudgeRawResponse(ctx context.Context, artifactDir, fileName str 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 diff --git a/internal/judge/agent_judge_test.go b/internal/judge/agent_judge_test.go index aac60e23..f932c01b 100644 --- a/internal/judge/agent_judge_test.go +++ b/internal/judge/agent_judge_test.go @@ -968,7 +968,7 @@ func TestAgentJudge_CorrectionRetryFallbackSucceeds(t *testing.T) { //nolint:cyc t.Fatalf("expected one serialized fallback user message, got %#v", ag.allMessages) } fallback := ag.allMessages[1][0] - if fallback.Role != transcript.RoleUser { + if fallback.Role != transcript.RoleUser || fallback.Turn != 1 { t.Fatalf("unexpected fallback role: %#v", fallback) } encodedFirstOutput, err := json.Marshal(firstOutput) @@ -1053,7 +1053,8 @@ func TestAgentJudge_ResumerWithoutSessionIDUsesFallbackHistory(t *testing.T) { 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 { + 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) } }