fix(llm): support opt-in OpenAI streaming

[zhouzhih]

Summary

This PR adds an opt-in streamed Chat Completions path for OpenAI-compatible providers whose gateways require SSE responses.

Fixes #358.

Changes

• select the SDK streaming API only when extra_body.stream is the JSON boolean true
• accumulate streamed text, fragmented tool calls, finish reasons, usage, and vendor reasoning_content into the existing ChatResponse
• preserve cached-token usage from raw usage chunks without injecting stream_options
• propagate stream errors and cancellation without returning partial responses or retrying the logical completion
• reject inconsistent, empty, and cleanly truncated streams
• add real httptest.Server coverage for the streaming request and response contracts

Configuration

Streaming remains opt-in through the existing provider configuration:

ocr config set custom_providers.<name>.extra_body '{"stream":true}'

The value must be a JSON boolean. A missing value, false, or the string "true" keeps the existing non-streaming path.

Non-goals

• This does not enable streaming by default or print tokens incrementally in the CLI.
• This does not add automatic fallback between streaming and non-streaming requests.
• This does not change Anthropic behavior, public LLM interfaces, or provider schemas.

Validation

• go test ./internal/llm -run 'TestOpenAIClient_(StreamOnlyGateway|Streaming.*)$' -count=1
• go test ./internal/llm -count=1
• go test ./cmd/opencodereview ./internal/llm -count=1
• go vet ./...
• go build -o /tmp/ocr-issue-358 ./cmd/opencodereview
• git diff --check origin/main...HEAD

go test ./... -count=1 and make test also pass all changed and dependent packages, including internal/llm. On this machine they still report the clean-origin/main internal/tool failures because Apple Git 2.39.3 does not support git grep --end-of-options (fatal: unable to resolve revision: --end-of-options).

[zhouzhih]

@lizhengfeng101 Could you please review this PR when you have time?

It fixes #358 by adding an opt-in SDK streaming path for OpenAI-compatible providers while preserving the existing non-streaming default.

[github-actions]

🔍 OpenCodeReview found 2 issue(s) in this PR.

• ✅ Successfully posted inline: 2 comment(s)

[github-actions]

Performance concern: resolveUsage performs a full JSON unmarshal (json.Unmarshal into map[string]any) on every streaming chunk. In typical OpenAI streaming responses, usage information only appears in the final chunk (when stream_options.include_usage is set). Parsing every chunk's raw JSON is wasteful, especially for high-volume streams with many tokens.

Consider only attempting to resolve usage from the last chunk, or at minimum checking for the presence of a "usage" key in the raw JSON string before doing the full unmarshal (e.g., a simple strings.Contains(chunk.RawJSON(), "\"usage\"") guard).

[zhouzhih]

Fixed in d5df669. Usage resolution is now gated by the SDK field metadata chunk.JSON.Usage.Valid(), so omitted or null usage chunks skip resolveUsage JSON unmarshalling while the final usage object still preserves cached-token fields. The usage fixture now exercises usage:null token chunks followed by the final usage object. Verified with the streaming tests and race detector.

[github-actions]

Potential error masking: When accumulator.AddChunk(chunk) returns false, the function returns a generic "inconsistent chunks" error without first checking stream.Err(). While the SDK documents that AddChunk returns false for inconsistent data, there may be cases where an underlying stream error coincides with or causes accumulation failure. Checking stream.Err() before returning would provide better diagnostics:

if !accumulator.AddChunk(chunk) {
    if err := stream.Err(); err != nil {
        return nil, fmt.Errorf("OpenAI streaming error during accumulation: %w", err)
    }
    return nil, fmt.Errorf("OpenAI streaming response contained inconsistent chunks")
}

[zhouzhih]

Kept the existing ordering after checking openai-go v3.41.0. Entering the loop means stream.Next() returned true, which only happens after the current event has decoded successfully; stream.Err() is therefore nil at this point. Decode, SSE, transport, and cancellation errors make Next() return false and are returned by the existing post-loop Err() check. AddChunk false specifically reports accumulator inconsistency, so checking Err() there would add an unreachable branch.

[zhouzhih]

Addressed the two review threads in d5df669:

• Gated streaming usage resolution with chunk.JSON.Usage.Valid(), so omitted or null token chunks avoid a full JSON unmarshal and the final usage object still preserves cached-token fields.
• Kept the AddChunk error ordering: openai-go only enters the loop after successful decode, while stream errors are surfaced by the existing post-loop Err() check.
• Updated the usage fixture to cover usage:null chunks followed by the final usage object.

Local verification:

• go test ./internal/llm -count=1
• go test -race ./internal/llm -run TestOpenAIClient_Streaming -count=1
• go test ./cmd/... ./internal/llm -count=1
• go vet ./...
• go build ./...

Reviewer manual gateway regression

Live gateway status: not run in this environment because no stream-enabled OpenAI-compatible provider credentials were available. The steps below are a reviewer checklist, not a validation claim.

Prerequisite: use an OpenAI-compatible gateway that rejects non-streaming Chat Completions requests but accepts the JSON boolean stream: true. If the gateway accepts both modes, the negative control will not reproduce #358.

Set up an isolated config so the check does not read or modify the reviewer's normal OCR configuration:

go build -o /tmp/ocr-pr359 ./cmd/opencodereview
tmp_home=$(mktemp -d)

export STREAM_GATEWAY_URL="https://gateway.example.com/v1"
export STREAM_GATEWAY_MODEL="<model>"
read -rsp "Gateway API key: " STREAM_GATEWAY_API_KEY; echo

HOME="$tmp_home" /tmp/ocr-pr359 config set custom_providers.stream-only.url "$STREAM_GATEWAY_URL"
HOME="$tmp_home" /tmp/ocr-pr359 config set custom_providers.stream-only.protocol openai
HOME="$tmp_home" /tmp/ocr-pr359 config set custom_providers.stream-only.model "$STREAM_GATEWAY_MODEL"
HOME="$tmp_home" /tmp/ocr-pr359 config set custom_providers.stream-only.api_key "$STREAM_GATEWAY_API_KEY"
HOME="$tmp_home" /tmp/ocr-pr359 config set provider stream-only

Negative control — confirm the existing non-streaming path still reproduces the gateway failure:

HOME="$tmp_home" /tmp/ocr-pr359 config set custom_providers.stream-only.extra_body '{"stream":false}'
HOME="$tmp_home" /tmp/ocr-pr359 llm test

Expected on a stream-only gateway: the request fails with 400 Bad Request or the gateway's equivalent non-stream rejection.

Opt-in path — enable real SSE handling:

HOME="$tmp_home" /tmp/ocr-pr359 config set custom_providers.stream-only.extra_body '{"stream":true}'
HOME="$tmp_home" /tmp/ocr-pr359 llm test

Expected: exit code 0, non-empty response content, and ✓ Connection test successful. The value must be the JSON boolean true; the string "true" intentionally stays on the non-streaming path.

End-to-end review on a small real commit:

cd /path/to/a/test-repository
HOME="$tmp_home" /tmp/ocr-pr359 review --commit HEAD --format json

Expected: the review completes and emits review output without the gateway 400 or an SSE/JSON decode error.

Clean up the temporary config and binary:

unset STREAM_GATEWAY_API_KEY
rm -rf "$tmp_home" /tmp/ocr-pr359

If results are posted back to the PR, redact gateway URLs and credentials.