Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,15 @@ export OCR_LLM_MODEL=claude-opus-4-6
export OCR_USE_ANTHROPIC=true
```

For OpenAI Codex / GPT-5 reasoning models, use the Responses endpoint:

```bash
ocr config set llm.url https://api.openai.com/v1/responses
ocr config set llm.auth_token "$OPENAI_API_KEY"
ocr config set llm.model gpt-5.1-codex-max
ocr config set llm.use_anthropic false
```

Config is stored in `~/.opencodereview/config.json`.

It is also compatible with Claude Code environment variables (`ANTHROPIC_BASE_URL`, `ANTHROPIC_AUTH_TOKEN`, `ANTHROPIC_MODEL`) and parses `~/.zshrc` / `~/.bashrc` for those exports.
Expand Down Expand Up @@ -331,9 +340,9 @@ Config file: `~/.opencodereview/config.json`

| Key | Type | Example |
|-----|------|---------|
| `llm.url` | string | `https://api.openai.com/v1/chat/completions` |
| `llm.url` | string | `https://api.openai.com/v1/responses` or `https://api.openai.com/v1/chat/completions` |
| `llm.auth_token` | string | `sk-xxxxxxx` |
| `llm.model` | string | `claude-opus-4-6` |
| `llm.model` | string | `gpt-5.1-codex-max` or `claude-opus-4-6` |
| `llm.use_anthropic` | boolean | `true` \| `false` |
| `language` | string | `English` \| `Chinese` (default: Chinese) |
| `telemetry.enabled` | boolean | `true` \| `false` |
Expand All @@ -352,6 +361,8 @@ Environment variables take precedence over the config file.
| `OCR_LLM_MODEL` | Model name |
| `OCR_USE_ANTHROPIC` | `true` = Anthropic, `false` = OpenAI |

`OCR_LLM_AUTH_TOKEN` and `OCR_LLM_USE_ANTHROPIC` are also accepted for compatibility with the CI examples.


## Telemetry

Expand Down
5 changes: 3 additions & 2 deletions cmd/opencodereview/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,9 +247,10 @@ Usage:
ocr config set <key> <value>

Examples:
ocr config set llm.url https://xx/v1/openai/chat/completions
ocr config set llm.url https://api.openai.com/v1/responses
ocr config set llm.auth_token xxxxxxxxxx
ocr config set llm.model claude-opus-4-6
ocr config set llm.model gpt-5.1-codex-max
ocr config set llm.use_anthropic false
ocr config set llm.extra_body '{"thinking":{"type":"disabled"}}'
ocr config set language English
ocr config set telemetry.enabled true
Expand Down
62 changes: 49 additions & 13 deletions internal/llm/client.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Package llm provides LLM client interfaces supporting multiple protocols.
// Supported protocols: Anthropic Messages API, OpenAI Chat Completions API.
// Supported protocols: Anthropic Messages API, OpenAI Chat Completions API,
// and OpenAI Responses API.
package llm

import (
Expand Down Expand Up @@ -197,7 +198,8 @@ type ClientConfig struct {
// --- Factory ---

// NewLLMClient creates the appropriate client based on the resolved endpoint protocol.
// protocol: "anthropic" -> AnthropicClient, anything else -> OpenAIClient.
// protocol: "anthropic" -> AnthropicClient; OpenAI /responses URLs -> OpenAIResponsesClient;
// anything else -> OpenAIClient.
func NewLLMClient(ep ResolvedEndpoint) LLMClient {
cfg := ClientConfig{
URL: ep.URL,
Expand All @@ -208,6 +210,9 @@ func NewLLMClient(ep ResolvedEndpoint) LLMClient {
if ep.Protocol == "anthropic" {
return NewAnthropicClient(cfg)
}
if isResponsesEndpoint(ep.URL) {
return NewOpenAIResponsesClient(cfg)
}
return NewOpenAIClient(cfg)
}

Expand Down Expand Up @@ -270,7 +275,11 @@ func CountTokensForModel(text string, modelName string) int {
func encodingForModel(modelName string) string {
lower := strings.ToLower(modelName)
switch {
case strings.Contains(lower, "o1") || strings.Contains(lower, "o3") || strings.Contains(lower, "o4"):
case strings.Contains(lower, "gpt-5") ||
strings.Contains(lower, "codex") ||
strings.Contains(lower, "o1") ||
strings.Contains(lower, "o3") ||
strings.Contains(lower, "o4"):
return "o200k_base"
default:
return "cl100k_base"
Expand Down Expand Up @@ -307,6 +316,19 @@ func NewClient(cfg ClientConfig) *OpenAIClient {
return NewOpenAIClient(cfg)
}

func isResponsesEndpoint(rawURL string) bool {
return strings.HasSuffix(strings.TrimRight(rawURL, "/"), "/responses")
}
Comment on lines +319 to +321
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isResponsesEndpoint 对含查询参数的 URL 匹配可能失败

该函数使用 strings.HasSuffix 检查 URL 是否以 /responses 结尾,但未考虑 URL 可能携带查询参数(如 https://example.com/v1/responses?api-version=2024-01)或片段标识符的情况。这会导致本应路由到 OpenAIResponsesClient 的请求被错误地路由到 OpenAIClient

建议:使用 net/url 包先解析 URL,基于路径部分进行匹配:

func isResponsesEndpoint(rawURL string) bool {
    u, err := url.Parse(rawURL)
    if err != nil {
        return strings.HasSuffix(strings.TrimRight(rawURL, "/"), "/responses")
    }
    return strings.HasSuffix(strings.TrimRight(u.Path, "/"), "/responses")
}


func useMaxCompletionTokens(model string) bool {
lower := strings.ToLower(model)
return strings.Contains(lower, "gpt-5") ||
strings.Contains(lower, "codex") ||
strings.Contains(lower, "o1") ||
strings.Contains(lower, "o3") ||
strings.Contains(lower, "o4")
}
Comment on lines +323 to +330
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DRY 违规:encodingForModeluseMaxCompletionTokens 中的模型匹配逻辑完全重复

这两个函数都维护着相同的模型名称匹配条件 (gpt-5, codex, o1, o3, o4)。未来新增模型时,如果只更新了其中一个函数而遗漏另一个,将导致 token 计数与实际请求参数不一致。

建议:将模型匹配逻辑提取为一个共享的判断函数,例如:

func isAdvancedReasoningModel(modelName string) bool {
    lower := strings.ToLower(modelName)
    return strings.Contains(lower, "gpt-5") ||
        strings.Contains(lower, "codex") ||
        strings.Contains(lower, "o1") ||
        strings.Contains(lower, "o3") ||
        strings.Contains(lower, "o4")
}

然后在 encodingForModeluseMaxCompletionTokens 中复用该函数。


// ChatRequest represents the payload for a chat completion call.
type ChatRequest struct {
Model string `json:"model"`
Expand Down Expand Up @@ -370,15 +392,10 @@ func (c *OpenAIClient) StreamCompletionWithCtx(ctx context.Context, req ChatRequ
}

return c.withRetryCtx(ctx, func() error {
body := make(map[string]any)
b, _ := json.Marshal(req)
json.Unmarshal(b, &body)
body["model"] = model
for k, v := range c.cfg.ExtraBody {
body[k] = v
payload, err := c.buildRequestPayload(model, req)
if err != nil {
return fmt.Errorf("marshal request body: %w", err)
}

payload, _ := json.Marshal(body)
httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, c.cfg.URL, bytes.NewReader(payload))
if err != nil {
return fmt.Errorf("create request: %w", err)
Expand Down Expand Up @@ -431,8 +448,7 @@ func (c *OpenAIClient) doRequestCtx(ctx context.Context, model string, req ChatR
if model == "" {
model = c.cfg.Model
}
req.Model = model
payload, err := mergeExtraBody(req, c.cfg.ExtraBody)
payload, err := c.buildRequestPayload(model, req)
if err != nil {
return nil, fmt.Errorf("marshal request body: %w", err)
}
Expand Down Expand Up @@ -478,6 +494,26 @@ func (c *OpenAIClient) doRequestCtx(ctx context.Context, model string, req ChatR
}, nil
}

func (c *OpenAIClient) buildRequestPayload(model string, req ChatRequest) ([]byte, error) {
req.Model = model
b, err := json.Marshal(req)
if err != nil {
return nil, err
}
var body map[string]any
if err := json.Unmarshal(b, &body); err != nil {
return nil, err
}
if req.MaxTokens > 0 && useMaxCompletionTokens(model) {
delete(body, "max_tokens")
body["max_completion_tokens"] = req.MaxTokens
}
for k, v := range c.cfg.ExtraBody {
body[k] = v
}
Comment on lines +507 to +513
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: ExtraBody 可能覆盖或撤销 max_completion_tokens 的转换

在此处先将 max_tokens 删除并替换为 max_completion_tokens,但随后在下方遍历 c.cfg.ExtraBody 时,用户配置的 ExtraBody 可能会重新写入 max_tokens 键,导致请求体中同时存在 max_tokensmax_completion_tokens。OpenAI API 对这两种字段同时存在时的行为未做明确保证,可能导致调用失败或行为异常。

建议:在 ExtraBody 合并之后再做 max_tokens -> max_completion_tokens 的转换,或者在 ExtraBody 合并后再次清理冲突字段。例如:

for k, v := range c.cfg.ExtraBody {
    body[k] = v
}
// 在 ExtraBody 合并之后再处理转换
if req.MaxTokens > 0 && useMaxCompletionTokens(model) {
    delete(body, "max_tokens")
    body["max_completion_tokens"] = req.MaxTokens
}

return json.Marshal(body)
}

// --- AnthropicClient ---

const anthropicVersion = "2023-06-01"
Expand Down
202 changes: 202 additions & 0 deletions internal/llm/client_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package llm

import (
"encoding/json"
"net/http"
"net/http/httptest"
"testing"
)

Expand Down Expand Up @@ -47,6 +50,205 @@ func TestNewOpenAIClient_URLNormalization(t *testing.T) {
}
}

func TestNewOpenAIResponsesClient_URLNormalization(t *testing.T) {
tests := []struct {
name string
inputURL string
wantURL string
}{
{
name: "base URL without trailing slash",
inputURL: "https://api.example.com/v1",
wantURL: "https://api.example.com/v1/responses",
},
{
name: "base URL with trailing slash",
inputURL: "https://api.example.com/v1/",
wantURL: "https://api.example.com/v1/responses",
},
{
name: "full URL already has responses",
inputURL: "https://api.example.com/v1/responses",
wantURL: "https://api.example.com/v1/responses",
},
{
name: "full URL with trailing slash",
inputURL: "https://api.example.com/v1/responses/",
wantURL: "https://api.example.com/v1/responses/",
},
{
name: "bare host",
inputURL: "https://api.example.com",
wantURL: "https://api.example.com/responses",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
client := NewOpenAIResponsesClient(ClientConfig{URL: tt.inputURL})
if client.cfg.URL != tt.wantURL {
t.Errorf("got URL %q, want %q", client.cfg.URL, tt.wantURL)
}
})
}
}

func TestNewLLMClient_SelectsResponsesClient(t *testing.T) {
client := NewLLMClient(ResolvedEndpoint{
URL: "https://api.openai.com/v1/responses",
Token: "test-token",
Model: "gpt-5.1-codex-max",
Protocol: "openai",
})
if _, ok := client.(*OpenAIResponsesClient); !ok {
t.Fatalf("expected *OpenAIResponsesClient, got %T", client)
}
}

func TestOpenAIClient_UsesMaxCompletionTokensForCodexModels(t *testing.T) {
var got map[string]any
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if err := json.NewDecoder(r.Body).Decode(&got); err != nil {
t.Fatalf("decode request: %v", err)
}
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte(`{"id":"chatcmpl_1","model":"gpt-5.1-codex-max","choices":[{"message":{"role":"assistant","content":"ok"},"finish_reason":"stop"}]}`))
}))
defer server.Close()

client := NewOpenAIClient(ClientConfig{URL: server.URL, APIKey: "test-token", Model: "gpt-5.1-codex-max"})
_, err := client.Completions(ChatRequest{
Messages: []Message{NewTextMessage("user", "hi")},
MaxTokens: 42,
})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}

if _, ok := got["max_tokens"]; ok {
t.Fatalf("request included max_tokens: %#v", got)
}
if got["max_completion_tokens"] != float64(42) {
t.Fatalf("max_completion_tokens = %#v, want 42", got["max_completion_tokens"])
}
}

func TestOpenAIClient_KeepsMaxTokensForLegacyModels(t *testing.T) {
var got map[string]any
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if err := json.NewDecoder(r.Body).Decode(&got); err != nil {
t.Fatalf("decode request: %v", err)
}
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte(`{"id":"chatcmpl_1","model":"gpt-4o","choices":[{"message":{"role":"assistant","content":"ok"},"finish_reason":"stop"}]}`))
}))
defer server.Close()

client := NewOpenAIClient(ClientConfig{URL: server.URL, APIKey: "test-token", Model: "gpt-4o"})
_, err := client.Completions(ChatRequest{
Messages: []Message{NewTextMessage("user", "hi")},
MaxTokens: 42,
})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}

if _, ok := got["max_completion_tokens"]; ok {
t.Fatalf("request included max_completion_tokens: %#v", got)
}
if got["max_tokens"] != float64(42) {
t.Fatalf("max_tokens = %#v, want 42", got["max_tokens"])
}
}

func TestOpenAIResponsesClient_RequestAndResponseMapping(t *testing.T) {
var got map[string]any
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/v1/responses" {
t.Fatalf("path = %q, want /v1/responses", r.URL.Path)
}
if err := json.NewDecoder(r.Body).Decode(&got); err != nil {
t.Fatalf("decode request: %v", err)
}
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte(`{
"id":"resp_1",
"model":"gpt-5.1-codex-max",
"output":[
{"type":"message","role":"assistant","content":[{"type":"output_text","text":"Need more context"}]},
{"type":"function_call","call_id":"call_1","name":"code_search","arguments":"{\"query\":\"foo\"}"}
],
"usage":{"input_tokens":10,"output_tokens":5,"total_tokens":15}
}`))
}))
defer server.Close()

client := NewOpenAIResponsesClient(ClientConfig{
URL: server.URL + "/v1",
APIKey: "test-token",
Model: "gpt-5.1-codex-max",
})
resp, err := client.Completions(ChatRequest{
Messages: []Message{
NewTextMessage("system", "review code"),
NewTextMessage("user", "diff"),
NewToolCallMessage("", []ToolCall{{
ID: "call_prev",
Type: "function",
Function: FunctionCall{
Name: "code_search",
Arguments: `{"query":"bar"}`,
},
}}),
NewToolResultMessage("call_prev", "result"),
},
Tools: []ToolDef{{
Type: "function",
Function: FunctionDef{
Name: "code_search",
Description: "Search code",
Parameters: map[string]any{"type": "object"},
},
}},
MaxTokens: 99,
})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}

if got["max_output_tokens"] != float64(99) {
t.Fatalf("max_output_tokens = %#v, want 99", got["max_output_tokens"])
}
input := got["input"].([]any)
if input[0].(map[string]any)["role"] != "system" {
t.Fatalf("first input item = %#v", input[0])
}
if input[2].(map[string]any)["type"] != "function_call" {
t.Fatalf("third input item = %#v", input[2])
}
if input[3].(map[string]any)["type"] != "function_call_output" {
t.Fatalf("fourth input item = %#v", input[3])
}
tools := got["tools"].([]any)
if tools[0].(map[string]any)["strict"] != false {
t.Fatalf("tool strict = %#v, want false", tools[0].(map[string]any)["strict"])
}

if resp.Content() != "Need more context" {
t.Fatalf("content = %q", resp.Content())
}
calls := resp.ToolCalls()
if len(calls) != 1 || calls[0].ID != "call_1" || calls[0].Function.Name != "code_search" {
t.Fatalf("tool calls = %#v", calls)
}
if resp.Usage == nil || resp.Usage.TotalTokens != 15 {
t.Fatalf("usage = %#v", resp.Usage)
}
}

func TestNewAnthropicClient_URLNormalization(t *testing.T) {
tests := []struct {
name string
Expand Down
Loading
Loading