fix(responses): accept scientific created_at timestamps - #6577
Open
AmPlace wants to merge 2 commits into
Open
Conversation
Contributor
WalkthroughThe PR adds ChangesTimestamp normalization
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@relay/channel/openai/relay_responses_scientific_test.go`:
- Around line 18-21: Update the test setup around gin.SetMode in the affected
test to capture the existing mode with gin.Mode() before switching to
gin.TestMode, then restore that saved mode in the existing t.Cleanup callback
alongside StreamingTimeout.
In `@relaykit/dto/values_test.go`:
- Around line 9-31: Add a MarshalJSON test for UnixTimestamp using the value
1768488160, and assert the serialized output is the exact unquoted integer JSON
representation "1768488160", not scientific notation.
In `@relaykit/dto/values.go`:
- Around line 75-80: Update UnixTimestamp.UnmarshalJSON to avoid Float64
conversion: parse the numeric token using exact decimal arithmetic, truncate
only after parsing, and reject values outside the int64 range before converting
to int64. Add boundary tests covering math.MaxInt64 and the first out-of-range
value.
- Around line 67-85: Update UnixTimestamp.UnmarshalJSON and
UnixTimestamp.MarshalJSON to use the common.Unmarshal and common.Marshal
wrappers instead of the direct json functions, and adjust imports to reference
the common package while retaining json.Number if still required.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3d3e6d00-da06-4381-a3be-2a804f4809d2
📒 Files selected for processing (10)
relay/channel/openai/chat_via_responses.gorelay/channel/openai/relay_responses_scientific_test.gorelaykit/dto/openai_compaction.gorelaykit/dto/openai_response.gorelaykit/dto/values.gorelaykit/dto/values_test.gorelaykit/relayconvert/internal/oai_chat/to_oai_responses_resp.gorelaykit/relayconvert/internal/oai_chat/to_oai_responses_stream_resp.gorelaykit/relayconvert/internal/oai_responses/to_oai_chat_resp.gorelaykit/relayconvert/internal/oai_responses/to_oai_chat_stream_resp.go
Comment on lines
+67
to
+85
| var n json.Number | ||
| if err := json.Unmarshal(trimmed, &n); err != nil { | ||
| return err | ||
| } | ||
| if i, err := n.Int64(); err == nil { | ||
| *t = UnixTimestamp(i) | ||
| return nil | ||
| } | ||
| f, err := n.Float64() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| *t = UnixTimestamp(int64(f)) | ||
| return nil | ||
| } | ||
|
|
||
| func (t UnixTimestamp) MarshalJSON() ([]byte, error) { | ||
| return json.Marshal(int64(t)) | ||
| } |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate relevant files =="
fd -a 'values\.go|json\.go' . | sed 's#^\./##'
echo
echo "== relaykit values outline =="
ast-grep outline relaykit/dto/values.go --view expanded || true
echo
echo "== relaykit values lines =="
cat -n relaykit/dto/values.go | sed -n '1,140p'
echo
echo "== common json outline =="
ast-grep outline common/json.go --view expanded || true
echo
echo "== common json lines =="
cat -n common/json.go | sed -n '1,240p'
echo
echo "== imports usages direct encoding/json Marshal/Unmarshal in common and relaykit dto =="
rg -n "encoding/json|json\.(Marshal|Unmarshal)|common/json" relaykit/dto values.go common -S || true
echo
echo "== package deps references relaykit -> common =="
sed -n '1,80p' relaykit/dto/values.go.pyRepository: QuantumNous/new-api
Length of output: 9968
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== search UnixTimestamp usages =="
rg -n "UnixTimestamp|Timestamp" relaykit common . --glob '!**/vendor/**' --glob '!**/.git/**' | head -200
echo
echo "== behavioral probe for JSON.Number unmarshal/Float64 edge cases using standard library semantics =="
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
cat > "$tmpdir/probe.go" <<'GO'
package main
import (
"encoding/json"
"fmt"
"math"
)
type UnixTimestamp int64
func main() {
inputs := []string{`4`, `4.9`, `9223372036854775807`, `9223372036854775808`, `9223372036854775807.5`, `9.223372036854776e18`, `1e18`, `-9223372036854775808`, `-9223372036854775809`, `-9.223372036854776e18`}
for _, s := range inputs {
var n json.Number
err := json.Unmarshal([]byte(s), &n)
if err != nil {
fmt.Printf("%s -> Unmarshal err: %v\n", s, err)
continue
}
i, err := n.Int64()
fmt.Printf("%s -> Int64=%v err=%v", s, i, err)
f, ferr := n.Float64()
if ferr == nil {
fmt.Printf(" Float64=%.17g cast=%d", f, int64(f))
} else {
fmt.Printf(" Float64 err=%v", ferr)
}
fmt.Println()
}
fmt.Println("IsNumber float literal parse behavior:")
parsed := json.Number("9007199254740993").String()
fmt.Println(parsed, float64Str(parsed), int64FromFloat(parsed))
}
func float64Str(s string) string {
f, err := json.Number(s).Float64()
if err != nil {
return "<error>"
}
return fmt.Sprintf("%.17g", f)
}
func int64FromFloat(s string) string {
f, err := json.Number(s).Float64()
if err != nil {
return "n/a"
}
return fmt.Sprintf("%d", int64(f))
}
GO
go run "$tmpdir/probe.go"Repository: QuantumNous/new-api
Length of output: 18449
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
cat > "$tmpdir/probe.go" <<'GO'
package main
import (
"encoding/json"
"fmt"
"math"
)
func main() {
inputs := []string{
`4`,
`4.9`,
`9223372036854775807`,
`9223372036854775808`,
`9223372036854775807.5`,
`9.223372036854776e18`,
`1e18`,
`-9223372036854775808`,
`-9223372036854775809`,
`-9.223372036854776e18`,
`9007199254740993`,
}
for _, s := range inputs {
var n json.Number
err := json.Unmarshal([]byte(s), &n)
if err != nil {
fmt.Printf("%s -> Unmarshal err: %v\n", s, err)
continue
}
i, err := n.Int64()
fmt.Printf("%s -> Int64=%v err=%v", s, i, err)
f, ferr := n.Float64()
if ferr == nil {
fSign, fMan, fExp := math.Frexp(f)
fmt.Printf(" Float64=%.17g sign=%g man=%.17g exp=%d cast=%d", f, fSign, fMan, fExp, int64(f))
} else {
fmt.Printf(" Float64 err=%v", ferr)
}
fmt.Println()
}
fmt.Println("IsNumber float literal parse behavior:")
n := json.Number("9007199254740993")
s := n.String()
f, _ := n.Float64()
fmt.Printf("String=%s Float64=%.0f int64=%d\n", s, f, int64(f))
}
GO
go run "$tmpdir/probe.go"Repository: QuantumNous/new-api
Length of output: 283
Use the JSON wrappers from common/json.go for UnixTimestamp.
UnixTimestamp.UnmarshalJSON and UnixTimestamp.MarshalJSON call json.Unmarshal and json.Marshal directly. Use the common.Unmarshal / common.Marshal wrappers for this app-level JSON path and adjust the import if needed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@relaykit/dto/values.go` around lines 67 - 85, Update
UnixTimestamp.UnmarshalJSON and UnixTimestamp.MarshalJSON to use the
common.Unmarshal and common.Marshal wrappers instead of the direct json
functions, and adjust imports to reference the common package while retaining
json.Number if still required.
Sources: Coding guidelines, Learnings
AmPlace
marked this pull request as ready for review
August 1, 2026 13:08
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
relay/channel/openai/relay_responses_scientific_test.go (1)
43-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse non-fatal assertions for independent checks.
Keep
require.Nilfor the API-error guard. Useassert.Containsandassert.Zerofor the independent response and stream-status checks. Add thetestify/assertimport.As per coding guidelines, new or substantially rewritten Go backend tests must use
testify/requirefor setup and fatal assertions andtestify/assertfor non-fatal checks.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@relay/channel/openai/relay_responses_scientific_test.go` around lines 43 - 48, Keep require.Nil as the fatal API-error guard in the test, but replace the independent response-body checks and stream-status count check with assert.Contains and assert.Zero. Add the testify/assert import while preserving the existing assertions and test flow.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@relay/channel/openai/relay_responses_scientific_test.go`:
- Around line 43-48: Keep require.Nil as the fatal API-error guard in the test,
but replace the independent response-body checks and stream-status count check
with assert.Contains and assert.Zero. Add the testify/assert import while
preserving the existing assertions and test flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 27c914cc-d68e-49fc-b5d9-03a54ae3d798
📒 Files selected for processing (3)
relay/channel/openai/relay_responses_scientific_test.gorelaykit/dto/values.gorelaykit/dto/values_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- relaykit/dto/values_test.go
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.
📝 变更描述 / Description
原生
/v1/responsesSSE handler 会先将每个data:解析为ResponsesStreamResponse，再转发给客户端。部分 Grok/xAI Responses 上游在response.completed.response.created_at中返回合法的科学计数法 JSON number，例如1.76848816E9。当前 DTO 使用int，解析失败后该终止事件被跳过，Codex 最终因没有收到response.completed而报告流提前关闭。本 PR 将 Responses 和 Responses Compaction 的
created_at改为可同时接受整数及科学计数法数字的UnixTimestamp，并同步当前relaykit转换层中的类型使用。输出仍序列化为整数 Unix 时间戳。新增测试覆盖：
created_at的response.completed。这是已关闭 PR #2946 针对当前
relaykit/dto目录结构的重新适配。🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
response.completed，但 scanner 继续到[DONE]。📸 运行证明 / Proof of Work
修复前的最小事件：
稳定产生：
修复后测试：
Summary by CodeRabbit
Bug Fixes
Tests