Skip to content

fix(relay): preserve presence/frequency penalty in Responses conversion - #6654

Merged
seefs001 merged 2 commits into
QuantumNous:mainfrom
UzkiS:fix/responses-penalty-passthrough
Aug 11, 2026
Merged

fix(relay): preserve presence/frequency penalty in Responses conversion#6654
seefs001 merged 2 commits into
QuantumNous:mainfrom
UzkiS:fix/responses-penalty-passthrough

Conversation

@UzkiS

@UzkiS UzkiS commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

dto.OpenAIResponsesRequest never declared presence_penalty / frequency_penalty. The non-passthrough path parses the request into that struct and re-marshals it, so both fields were silently dropped before reaching the upstream, while temperature / top_p survived because they are declared. Passthrough mode forwards the raw body and was unaffected.

  • Declare both fields as *float64 with omitempty so an explicit 0 is preserved and an absent field stays omitted.
  • Carry them across the Responses <-> Chat Completions converters, which rebuild the request field by field.
  • Clear them in the Codex adaptor next to temperature, since the Codex backend rejects sampling parameters it does not accept.

Note that the official OpenAI Responses API does not accept these two parameters; OpenAI-compatible upstreams such as vLLM do, and forwarding them is what the reporter's setup needs.

Fixes #6614

⚠️ 提交说明 / PR Notice

Important

  • 请提供人工撰写的简洁摘要,避免直接粘贴未经整理的 AI 输出。

📝 变更描述 / Description

/v1/responses 在非透传模式下会丢掉 presence_penaltyfrequency_penalty

根因是 dto.OpenAIResponsesRequest 从来没声明过这两个字段。非透传路径的流程是「按结构体解析请求 → 重新序列化发给上游」,结构体里没有的键在序列化那一步就没了;temperaturetop_p 因为声明了所以能正常到达。透传模式直接转发原始 body,所以不受影响 —— 这也解释了 issue 里三组对比的结果差异。

改动分三处:

  1. 补字段OpenAIResponsesRequest 加上这两个字段,类型用 *float64 + omitempty。用指针是为了区分「没传」和「显式传 0」—— 不传是 nil 不下发,传 0 是非 nil 照常下发;非指针配 omitempty 会把 0 静默吃掉。
  2. 补转换器:Responses ↔ Chat Completions 两个转换器是逐字段重建请求的,不补的话 Responses 请求落到只支持 chat 的渠道时照样丢,反方向同理。
  3. Codex 剥离:Codex 适配器里跟 temperature 一起置空。Codex 后端会拒绝它不认的采样参数,字段加进 DTO 后会开始转发出去,这里补上剥离以保持原有行为。

测试覆盖了显式零值不被 omitempty 吞掉、两个转换器方向的正值/零值/未设置三种情况,以及 Codex 的剥离。

本 PR 代码为 AI 辅助生成,已由提交者逐处审阅确认。

🚀 变更类型 / Type of change

  • 🐛 Bug 修复 (Bug fix) - 请关联对应 Issue,避免将设计取舍、理解偏差或预期不一致直接归类为 bug
  • ✨ 新功能 (New feature) - 重大特性建议先通过 Issue 沟通
  • ⚡ 性能优化 / 重构 (Refactor)
  • 📝 文档更新 (Documentation)

🔗 关联任务 / Related Issue

✅ 提交前检查项 / Checklist

  • 人工确认: 我已亲自整理并撰写此描述,没有直接粘贴未经处理的 AI 输出。
  • 非重复提交: 我已搜索现有的 IssuesPRs,确认不是重复提交。
  • Bug fix 说明: 若此 PR 标记为 Bug fix,我已提交或关联对应 Issue,且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。
  • 变更理解: 我已理解这些更改的工作原理及可能影响。
  • 范围聚焦: 本 PR 未包含任何与当前任务无关的代码改动。
  • 本地验证: 已在本地运行并通过测试或手动验证,维护者可以据此复核结果。
  • 安全合规: 代码中无敏感凭据,且符合项目代码规范。

Summary by CodeRabbit

  • Bug Fixes

    • Preserved frequency and presence penalty settings when converting between supported OpenAI request formats.
    • Explicit zero values and unset penalty fields are now handled correctly.
    • Removed unsupported penalty settings before forwarding Codex Responses requests.
    • Added clear validation errors for invalid penalty values.
  • Tests

    • Added coverage for positive, zero, unset, and invalid penalty values across request conversions.

`dto.OpenAIResponsesRequest` never declared `presence_penalty` /
`frequency_penalty`. The non-passthrough path parses the request into that
struct and re-marshals it, so both fields were silently dropped before
reaching the upstream, while `temperature` / `top_p` survived because they
are declared. Passthrough mode forwards the raw body and was unaffected.

- Declare both fields as `*float64` with `omitempty` so an explicit `0` is
  preserved and an absent field stays omitted.
- Carry them across the Responses <-> Chat Completions converters, which
  rebuild the request field by field.
- Clear them in the Codex adaptor next to `temperature`, since the Codex
  backend rejects sampling parameters it does not accept.

Note that the official OpenAI Responses API does not accept these two
parameters; OpenAI-compatible upstreams such as vLLM do, and forwarding them
is what the reporter's setup needs.

Fixes QuantumNous#6614
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 25bcbd9b-607f-4247-bcd0-a31344cf0408

📥 Commits

Reviewing files that changed from the base of the PR and between 9c4b036 and eacd875.

📒 Files selected for processing (6)
  • relay/channel/codex/adaptor_test.go
  • relaykit/dto/openai_request.go
  • relaykit/relayconvert/internal/oai_chat/to_oai_responses_req.go
  • relaykit/relayconvert/internal/oai_chat/to_oai_responses_req_test.go
  • relaykit/relayconvert/internal/oai_responses/to_oai_chat_req.go
  • relaykit/relayconvert/internal/oai_responses/to_oai_chat_req_test.go
🚧 Files skipped from review as they are similar to previous changes (5)
  • relaykit/relayconvert/internal/oai_chat/to_oai_responses_req_test.go
  • relaykit/relayconvert/internal/oai_responses/to_oai_chat_req.go
  • relaykit/relayconvert/internal/oai_chat/to_oai_responses_req.go
  • relay/channel/codex/adaptor_test.go
  • relaykit/relayconvert/internal/oai_responses/to_oai_chat_req_test.go

Walkthrough

The Responses request DTO now includes frequency and presence penalties. Chat Completions and Responses conversions preserve these fields, including explicit zero values. Codex conversion removes them with other unsupported parameters.

Changes

Penalty parameter handling

Layer / File(s) Summary
Request contract and bidirectional conversion
relaykit/dto/openai_request.go, relaykit/dto/openai_request_zero_value_test.go, relaykit/relayconvert/internal/oai_chat/..., relaykit/relayconvert/internal/oai_responses/...
OpenAIResponsesRequest adds optional penalty fields. Both conversion directions preserve positive, zero, and unset values. Serialization tests preserve explicit zeros.
Codex request filtering
relay/channel/codex/adaptor.go, relay/channel/codex/adaptor_test.go
Codex Responses conversion clears frequency and presence penalties. Tests verify that these fields and other unsupported parameters are removed.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ChatCompletionsRequest
  participant ChatToResponses
  participant OpenAIResponsesRequest
  participant ResponsesToChat
  ChatCompletionsRequest->>ChatToResponses: marshal penalty values
  ChatToResponses->>OpenAIResponsesRequest: set frequency_penalty and presence_penalty
  OpenAIResponsesRequest->>ResponsesToChat: provide raw JSON penalty values
  ResponsesToChat->>ChatCompletionsRequest: parse penalty values or return labeled errors
Loading

Possibly related PRs

Suggested reviewers: wxwwt

Poem

A rabbit checks each penalty field,
Zero values stay in the yield.
Responses carry numbers through,
Invalid JSON raises errors too.
Codex trims unsupported values.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the relay fix for preserving presence and frequency penalties during Responses conversion.
Linked Issues check ✅ Passed The changes satisfy issue [#6614] by preserving both penalties in Responses conversions, including zero values, and rejecting malformed values.
Out of Scope Changes check ✅ Passed All changes address penalty preservation, conversion validation, tests, or required Codex filtering related to the stated objective.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@seefs001

Copy link
Copy Markdown
Collaborator

新增的参数改成 json.RawMessage 类型吧

Change the two new fields on `dto.OpenAIResponsesRequest` from `*float64` to
`json.RawMessage`, matching the struct's convention for passthrough extension
parameters that the official OpenAI Responses API does not declare. Explicit
zero values still round-trip (a `RawMessage("0")` is never dropped by
`omitempty`), and the Codex adaptor's nil-clearing is unaffected.

The Responses->Chat converter now parses the raw scalar into `*float64` for
the chat request, returning an error on malformed input; the Chat->Responses
converter marshals the typed field back to raw bytes.
@UzkiS

UzkiS commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

新增的参数改成 json.RawMessage 类型吧

已修正,两个字段已改为 json.RawMessage,转换器里做了对应的 raw↔float64 转换,测试已通过。

顺带一提:AGENTS.md 里写的是 optional scalar fields MUST use pointer types with omitempty,而 json.RawMessage 底层是 []byte,不是指针类型,字面上和这条稍微有点出入。

@seefs001
seefs001 merged commit 253a74d into QuantumNous:main Aug 11, 2026
1 check passed
0401lucky pushed a commit to 0401lucky/new-api that referenced this pull request Aug 16, 2026
junjundesk pushed a commit to junjundesk/new-api that referenced this pull request Aug 17, 2026
DayFliggy pushed a commit to DayFliggy/Ren2Hub that referenced this pull request Aug 17, 2026
330079598 pushed a commit to 330079598/new-api that referenced this pull request Aug 19, 2026
kk66615 pushed a commit to kk66615/new-api that referenced this pull request Aug 22, 2026
上游 253a74d(QuantumNous#6654) 与 7d09c69(QuantumNous#6861) 新增的测试用例调用
ResponsesRequestToChatCompletionsRequest / ChatCompletionsRequestToResponsesRequest
时使用的是旧签名,而本地 PR QuantumNous#6655 补丁已为这两个函数引入
convmeta.Meta 首参(reasoning_content 跨转换保留的核心机制)。
合并后函数定义取本地版本、测试取上游版本,导致 relaykit 模块
vet 失败。为 5 处新增用例补上 nil meta。

受影响用例:
- to_oai_chat_req_test.go: penalty 转换 2 处
- to_oai_responses_req_test.go: prompt_cache_key 2 处 / penalty 1 处

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014AZbTUCaSkfaTLkrfzKGSP
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

/v1/responses 在非透传模式下转发时静默丢弃 presence_penalty / frequency_penalty

2 participants