Canonical BCP-47 casing in verbose logs; always request OpenAI stream usage#51
Merged
Merged
Conversation
…stream usage Verbose diagnostics now render language tags in conventional casing (zh-tw -> zh-TW, zh-hant -> zh-Hant) via a new canonicalLangTag helper, instead of the lowercase-normalized internal form. The OpenAI client now always sets stream_options.include_usage, since every documented compatible endpoint (OpenAI, Ollama, LM Studio) either supports it or ignores unknown fields; this drops the defaultEndpoint special-casing and its dead branch for custom base URLs. Also excludes _test.go files from the goconst linter and drops the resulting test-only string constants in main_test.go for readability.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
PR Summary by QodoCanonical BCP-47 casing in verbose logs; always request OpenAI stream usage
AI Description
Diagram
High-Level Assessment
Files changed (7)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
36 rules 1. Strict servers reject stream_options
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
101
to
110
| body := requestBody{ | ||
| Model: c.modelName, | ||
| Messages: []message{ | ||
| {Role: "system", Content: system}, | ||
| {Role: "user", Content: user}, | ||
| }, | ||
| Temperature: temperature, | ||
| Stream: true, | ||
| } | ||
|
|
||
| // stream_options.include_usage is an OpenAI extension. Only request it on | ||
| // the official endpoint; local/proxy servers reached via a custom base URL | ||
| // may reject unknown fields, so omit it there. | ||
| if c.defaultEndpoint { | ||
| body.StreamOptions = &streamOptions{IncludeUsage: true} | ||
| Temperature: temperature, | ||
| Stream: true, | ||
| StreamOptions: &streamOptions{IncludeUsage: true}, | ||
| } |
There was a problem hiding this comment.
1. Strict servers reject stream_options 🐞 Bug ☼ Reliability
openai.Client.Complete now always sends stream_options.include_usage, so OpenAI-compatible servers behind a custom MINT_BASE_URL that reject unknown fields will fail the request. The client surfaces the 4xx error and does not retry without stream_options, so translation breaks for those deployments.
Agent Prompt
### Issue description
The OpenAI client now always includes `stream_options.include_usage` in requests. Some OpenAI-compatible servers (often used via `MINT_BASE_URL`) can reject unknown fields; when that happens the request fails and the client does not attempt a fallback retry.
### Issue Context
- `requestBody.StreamOptions` is a pointer with `omitempty`; setting it non-nil always sends the JSON field.
- The project explicitly supports `MINT_BASE_URL` mode (proxy/local server).
- Tests state 400s are surfaced “as-is” with no recovery.
### Fix Focus Areas
- internal/provider/openai/openai.go[91-135]
- internal/provider/openai/openai_test.go[141-196]
### Suggested approach
- On non-200 responses (especially 400), detect the specific “unknown field”/schema error pattern (if present) and retry exactly once with `StreamOptions=nil`.
- Keep the original error if the retry also fails (or if the error is not a schema/unknown-field type).
- Add/adjust a unit test: server returns 400 when `stream_options` present, returns 200/stream when absent; assert two requests max and successful completion.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
zh-tw→zh-TW,zh-hant→zh-Hant) via a newcanonicalLangTaghelper, instead of the lowercase-normalized internal form used for matching.stream_options.include_usage, since every documented compatible endpoint (OpenAI, Ollama, LM Studio) either supports it or ignores unknown fields — removes thedefaultEndpointspecial-casing that previously omitted it for custom base URLs._test.gofiles from thegoconstlinter and drops the resulting test-only string constants inmain_test.go.docs/manual-testing.mdwith the new casing behavior, anMINT_BASE_URL-without-API-key example,MINT_VERBOSEenv var section, and a Ctrl+C/SIGTERM timing note.Test plan
make checkpasses (pre-commit hook re-ran it)TestCanonicalLangTag,TestCompleteRequestsStreamOptions(renamed from the old omit-for-custom-base-URL test),TestCompleteSurfaces400