Support Claude CLI as judge - #195
Conversation
Adds support for using Claude Code CLI as an LLM judge alongside the existing OpenAI judge. The implementation reuses the existing 'claude' binary instead of adding the Anthropic SDK dependency. - Add judge type configuration (openai/claude) - Implement Claude judge using CLI execution - Update documentation with Claude configuration examples - Add example eval config for Claude judge - Defaults to OpenAI for backward compatibility Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> # Conflicts: # pkg/llmjudge/llmjudge.go
Add unit tests covering: - Judge type selection and defaults - Claude judge creation with/without claude binary - Claude judge evaluation with various JSON responses - Error handling for malformed JSON and CLI errors - Configuration validation for both OpenAI and Claude judges Also simplify Claude judge configuration: - Remove requirement for API key and model name env vars - Claude judge only requires JUDGE_TYPE="claude" to be set - Update documentation to reflect simplified configuration Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR extends the LLM judge system to support Claude (Anthropic) as an alternative to OpenAI. It adds judge type configuration, runtime judge selection logic, Claude CLI integration for evaluation, documentation updates, and comprehensive test coverage for both execution paths. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant LLMJudge Factory
participant Config
participant OpenAI Judge
participant Claude Judge
participant Claude CLI
participant OpenAI API
Client->>LLMJudge Factory: NewLLMJudge(config)
LLMJudge Factory->>Config: Type()
Config-->>LLMJudge Factory: Judge type (OpenAI/Claude)
alt Type == Claude
LLMJudge Factory->>Claude Judge: Create with CLI check
Claude Judge-->>LLMJudge Factory: Instance
else Type == OpenAI
LLMJudge Factory->>OpenAI Judge: Create with API config
OpenAI Judge-->>LLMJudge Factory: Instance
end
Client->>Claude Judge: EvaluateText(prompt)
Claude Judge->>Claude CLI: Execute claude --print
Claude CLI-->>Claude Judge: JSON output
Claude Judge-->>Client: LLMJudgeResult
Client->>OpenAI Judge: EvaluateText(prompt)
OpenAI Judge->>OpenAI API: POST /completions
OpenAI API-->>OpenAI Judge: JSON response
OpenAI Judge-->>Client: LLMJudgeResult
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@pkg/llmjudge/config.go`:
- Around line 44-53: The Type() method on LLMJudgeEvalConfig can panic if cfg or
cfg.Env is nil; update LLMJudgeEvalConfig.Type() to defensively check for cfg ==
nil or cfg.Env == nil and return JudgeTypeOpenAI as the safe default, and only
then read cfg.Env.TypeKey and the environment variable; ensure the method still
preserves the existing behavior of falling back to JudgeTypeOpenAI when TypeKey
is empty or the env var is unset.
In `@pkg/llmjudge/llmjudge_test.go`:
- Around line 253-258: The mock script builds scriptContent using exitCode but
mistakenly converts it with string(rune(exitCode)); update the construction in
the test to convert exitCode to its decimal string representation (e.g., via
strconv.Itoa(exitCode) or fmt.Sprintf("%d", exitCode)) so the script contains
"exit 1" etc.; modify the code that appends the exit line (the block referencing
scriptContent and exitCode) to use the correct integer-to-string conversion.
🧹 Nitpick comments (1)
pkg/llmjudge/llmjudge.go (1)
261-268: Use stdin to pass large prompts instead of command-line arguments to avoid OS limits.The full prompt is currently passed as a single command-line argument. For very long agent responses or prompts, this could exceed OS command-line length limits (commonly 128KB-2MB depending on OS). Claude Code CLI supports reading prompts from stdin:
♻️ Proposed refactor using stdin
// Execute Claude Code CLI - cmd := exec.CommandContext(ctx, "claude", "--print", fullPrompt) + cmd := exec.CommandContext(ctx, "claude", "--print") cmd.Env = os.Environ() + cmd.Stdin = strings.NewReader(fullPrompt) outputBytes, err := cmd.CombinedOutput()
| func (cfg *LLMJudgeEvalConfig) Type() string { | ||
| if cfg.Env.TypeKey == "" { | ||
| return JudgeTypeOpenAI // default to openai for backward compatibility | ||
| } | ||
| judgeType := os.Getenv(cfg.Env.TypeKey) | ||
| if judgeType == "" { | ||
| return JudgeTypeOpenAI // default to openai if env var not set | ||
| } | ||
| return judgeType | ||
| } |
There was a problem hiding this comment.
Potential nil pointer dereference in Type() method.
If cfg.Env is nil, accessing cfg.Env.TypeKey on line 45 will panic. While NewLLMJudge validates that cfg.Env is not nil before calling cfg.Type(), this method is public and could be called directly by other code.
Consider adding a nil check for defensive programming:
🛡️ Proposed fix
func (cfg *LLMJudgeEvalConfig) Type() string {
+ if cfg.Env == nil {
+ return JudgeTypeOpenAI
+ }
if cfg.Env.TypeKey == "" {
return JudgeTypeOpenAI // default to openai for backward compatibility
}🤖 Prompt for AI Agents
In `@pkg/llmjudge/config.go` around lines 44 - 53, The Type() method on
LLMJudgeEvalConfig can panic if cfg or cfg.Env is nil; update
LLMJudgeEvalConfig.Type() to defensively check for cfg == nil or cfg.Env == nil
and return JudgeTypeOpenAI as the safe default, and only then read
cfg.Env.TypeKey and the environment variable; ensure the method still preserves
the existing behavior of falling back to JudgeTypeOpenAI when TypeKey is empty
or the env var is unset.
| scriptContent := "#!/bin/bash\n" | ||
| if exitCode != 0 { | ||
| scriptContent += "exit " + string(rune(exitCode)) + "\n" | ||
| } else { | ||
| scriptContent += "cat << 'EOF'\n" + output + "\nEOF\n" | ||
| } |
There was a problem hiding this comment.
Bug: Incorrect exit code conversion in mock script.
Line 255 uses string(rune(exitCode)) which converts the exit code to its corresponding Unicode character, not its string representation. For exitCode=1, this produces ASCII SOH (Start of Heading, a non-printable character), not the string "1".
This bug affects the "command execution error" test case, which may not be testing what it intends.
🐛 Proposed fix
+ "strconv"
)
// mockClaudeCommand creates a temporary mock script that simulates claude CLI output
func mockClaudeCommand(t *testing.T, output string, exitCode int) string {
t.Helper()
tmpDir := t.TempDir()
mockScript := tmpDir + "/claude"
scriptContent := "#!/bin/bash\n"
if exitCode != 0 {
- scriptContent += "exit " + string(rune(exitCode)) + "\n"
+ scriptContent += "exit " + strconv.Itoa(exitCode) + "\n"
} else {
scriptContent += "cat << 'EOF'\n" + output + "\nEOF\n"
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| scriptContent := "#!/bin/bash\n" | |
| if exitCode != 0 { | |
| scriptContent += "exit " + string(rune(exitCode)) + "\n" | |
| } else { | |
| scriptContent += "cat << 'EOF'\n" + output + "\nEOF\n" | |
| } | |
| scriptContent := "#!/bin/bash\n" | |
| if exitCode != 0 { | |
| scriptContent += "exit " + strconv.Itoa(exitCode) + "\n" | |
| } else { | |
| scriptContent += "cat << 'EOF'\n" + output + "\nEOF\n" | |
| } |
🤖 Prompt for AI Agents
In `@pkg/llmjudge/llmjudge_test.go` around lines 253 - 258, The mock script builds
scriptContent using exitCode but mistakenly converts it with
string(rune(exitCode)); update the construction in the test to convert exitCode
to its decimal string representation (e.g., via strconv.Itoa(exitCode) or
fmt.Sprintf("%d", exitCode)) so the script contains "exit 1" etc.; modify the
code that appends the exit line (the block referencing scriptContent and
exitCode) to use the correct integer-to-string conversion.
Cali0707
left a comment
Cali0707
left a comment
There was a problem hiding this comment.
Hey @janisz thanks for working on this! We are actually in the process of moving away from the bash exec of agents, in favour of using the Agent Client Protocol to standardize the communication.
With that in mind, would you be open to updating this PR to tackle #107 - essentially instead of making a claude code specific wrapper, we would make an ACP compatible implementation, and then just use the claude code ACP bits when users want to use claude code for the judge
|
Ah, you're right. With ACP it will be simplified. I'll close this PR then. I need to read more on ACP before I can commit to do it. |
2 participants
This PR adds
JUDGE_TYPEoption that allows using claude-cli as a judge. It does not support API calls to claude.Summary by CodeRabbit
New Features
Documentation