Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23163a8d5a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
internal/proxy/proxy.go
Outdated
| if ok { | ||
| return ring |
There was a problem hiding this comment.
updateKeyInfo() refreshes RequestPerMinute from DB, but this helper returns the first ring forever for a key and never compares the new RPM value. If an operator tightens or relaxes a key’s rate limit, the old limit keeps being enforced until process restart, which can either allow excess traffic or incorrectly throttle valid requests.
Useful? React with 👍 / 👎.
internal/proxy/proxy.go
Outdated
| c.Set("skipSpend", true) | ||
| return resp.StatusCode, false, nil |
There was a problem hiding this comment.
This path unconditionally marks streamed requests with skipSpend, and logSpendMiddleware returns early when that flag is set, so streamed completions never reach CreateSpendRecord/CreateKeySpendRecord. Any client using stream=true can therefore consume tokens without spend logs or balance deduction.
Useful? React with 👍 / 👎.
internal/proxy/proxy.go
Outdated
| if streamFlag, ok := c.Get("isStreamRequest"); ok { | ||
| if isStream, ok := streamFlag.(bool); ok && isStream { | ||
| return true |
There was a problem hiding this comment.
Treating stream=true in the request as sufficient to enter streaming mode means JSON error responses are also handled as streamed. Because this happens before the >=500 retry branch, a transient upstream 5xx on a streamed request will not retry other backends, reducing availability for the most common chat path.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56181dd91a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if streamErr != nil { | ||
| return http.StatusBadGateway, true, streamErr |
There was a problem hiding this comment.
In the streaming branch, streamToClient may already flush SSE bytes before returning an error, but this code returns shouldRetry=true; HandleRequest then retries another upstream and attempts to write a second response on the same gin.Context. When an upstream stream drops mid-response (e.g., broken SSE connection), clients can receive corrupted/duplicated output because headers/body have already been sent and cannot be safely retried.
Useful? React with 👍 / 👎.
| ch = make(chan float64, 100) | ||
| return | ||
| } | ||
| spend := c.MustGet("spend").(float64) |
There was a problem hiding this comment.
CreateKeySpendRecord unconditionally calls c.MustGet("spend"), but CreateSpendRecord now has multiple early-return paths (invalid/malformed upstream payload, type mismatch, missing model price) that skip setting this key. In those cases the middleware path panics during spend logging, turning an upstream-format/config issue into a 500 for the request.
Useful? React with 👍 / 👎.
1 participant
No description provided.