修复 Responses 非透传模式下工具原语保真#1703
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to capture and replay provider-specific sidecar data for the OpenAI Responses API, ensuring high fidelity when converting between internal and external request formats. It adds a ProviderExtensions field to the llm.Request and implements logic to handle raw tools, tool choices, and input items that are not structurally represented in the core model. The review feedback highlights that RawInputItems are currently captured but not replayed in the outbound request. Additionally, improvements were suggested for the RawToolChoice replay logic to prevent replaying stale data when a tool choice has been intentionally cleared or modified.
| func marshalRequestPayload(payload Request, llmReq *llm.Request) ([]byte, error) { | ||
| body, err := json.Marshal(payload) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| requestExt := openAIResponsesRequestExtensions(llmReq) | ||
| if requestExt == nil { | ||
| return body, nil | ||
| } | ||
|
|
||
| var obj map[string]json.RawMessage | ||
| if err := json.Unmarshal(body, &obj); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| if tools, ok := mergeRawOnlyTools(obj["tools"], requestExt); ok { | ||
| toolsRaw, err := json.Marshal(tools) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| obj["tools"] = toolsRaw | ||
| } | ||
|
|
||
| if len(requestExt.RawToolChoice) > 0 && rawToolChoiceMatchesCurrentTools(requestExt.RawToolChoice, payload.ToolChoice) { | ||
| obj["tool_choice"] = cloneRaw(requestExt.RawToolChoice) | ||
| } | ||
|
|
||
| return json.Marshal(obj) | ||
| } |
There was a problem hiding this comment.
The RawInputItems captured during the inbound phase (line 19) are not being replayed here in marshalRequestPayload. This means that any provider-specific input items that AxonHub doesn't structurally represent (and thus skips during inbound conversion) will be missing from the outbound request. To ensure full fidelity as stated in the PR description, consider implementing a merge logic for input items similar to how tools are handled.
| if current == nil { | ||
| return true | ||
| } |
There was a problem hiding this comment.
Returning true when current is nil causes the raw tool choice to be replayed even if it was intentionally cleared from the request (e.g., by a middleware). Since RawToolChoice is only populated if a tool choice existed in the original inbound request, a nil value in the current request should be respected as an instruction to remove it.
| if current == nil { | |
| return true | |
| } | |
| if current == nil { | |
| return false | |
| } |
| if currentSignature == "" { | ||
| return true | ||
| } |
There was a problem hiding this comment.
Returning true when currentSignature is empty is too permissive. If the structured tool choice was modified to a state that doesn't produce a recognized signature (but is not nil), replaying the raw tool choice might override that change with stale data. It's safer to only replay when signatures explicitly match.
| if currentSignature == "" { | |
| return true | |
| } | |
| if currentSignature == "" { | |
| return false | |
| } |
Greptile SummaryThis PR adds a
Confidence Score: 3/5Safe to merge for the tools and tool_choice replay paths, but the input-item preservation path is incomplete — items are captured but never re-injected on the outbound side. The raw-tool and raw-tool_choice replay logic is well-structured and protected by a signature guard that prevents stale data from being applied when the tool list changes. However, llm/transformer/openai/responses/request_extensions.go — the Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant InboundTransformer
participant attachExtensions
participant Pipeline
participant OutboundTransformer
participant marshalPayload
Client->>InboundTransformer: HTTP Request (raw JSON body)
InboundTransformer->>InboundTransformer: json.Unmarshal → Request struct
InboundTransformer->>attachExtensions: convertToLLMRequest(req, rawBody)
attachExtensions->>attachExtensions: parseRawRequestFragments(rawBody)
attachExtensions->>attachExtensions: buildRawOnlyToolFragments → RawTools
attachExtensions->>attachExtensions: buildRepresentedToolSignatures → ToolSignatures
attachExtensions->>attachExtensions: rawUnsupportedToolChoice → RawToolChoice
attachExtensions->>attachExtensions: buildRawOnlyInputFragments → RawInputItems (captured, not replayed)
attachExtensions-->>InboundTransformer: llm.Request with ProviderExtensions sidecar
InboundTransformer-->>Pipeline: llm.Request (normalised + sidecar)
Pipeline->>Pipeline: optional tool/message modification
Pipeline->>OutboundTransformer: llm.Request
OutboundTransformer->>OutboundTransformer: build Request payload (structured tools only)
OutboundTransformer->>marshalPayload: marshalRequestPayload(payload, llmReq)
marshalPayload->>marshalPayload: structuredToolSignaturesMatch?
alt signatures match
marshalPayload->>marshalPayload: mergeRawOnlyTools → insert raw tools at original indices
end
marshalPayload->>marshalPayload: rawToolChoiceMatchesCurrentTools?
alt choice unchanged
marshalPayload->>marshalPayload: replace tool_choice with raw RawToolChoice
end
marshalPayload-->>OutboundTransformer: merged JSON body
OutboundTransformer-->>Client: HTTP Request to upstream LLM
Reviews (1): Last reviewed commit: "修复 Responses 非透传工具原语保真" | Re-trigger Greptile |
| if len(requestExt.RawTools) == 0 && len(requestExt.RawToolChoice) == 0 && len(requestExt.RawInputItems) == 0 { | ||
| return | ||
| } |
There was a problem hiding this comment.
RawInputItems captured but never replayed
buildRawOnlyInputFragments populates requestExt.RawInputItems for unsupported input item types, and the guard on line 22 keeps the extension alive when only RawInputItems is non-empty. However, marshalRequestPayload merges back RawTools and RawToolChoice but has no corresponding step for RawInputItems. Any request that contains unsupported input item types (e.g. future OpenAI-specific items not in isStructurallyRepresentedInputItem) will have those items captured in the inbound, silently dropped during message conversion, and never re-injected on the outbound — defeating the stated goal of input-item fidelity for the non-passthrough path.
|
|
||
| // convertToLLMRequest converts OpenAI Responses API Request to llm.Request. | ||
| func convertToLLMRequest(req *Request) (*llm.Request, error) { | ||
| func convertToLLMRequest(req *Request, rawBody ...[]byte) (*llm.Request, error) { |
There was a problem hiding this comment.
rawBody ...[]byte signature technically accepts any number of []byte arguments but only ever uses the first. A caller that mistakenly passes two bodies (e.g. during a future refactor) would get silent partial behavior. Since convertToLLMRequest is a package-internal function, a plain rawBody []byte with a nil guard is clearer and safer.
| func convertToLLMRequest(req *Request, rawBody ...[]byte) (*llm.Request, error) { | |
| func convertToLLMRequest(req *Request, rawBody []byte) (*llm.Request, error) { |
| if len(rawBody) > 0 { | ||
| attachOpenAIResponsesRequestExtensions(chatReq, req, rawBody[0]) | ||
| } |
There was a problem hiding this comment.
convertToLLMRequest is changed to take a plain []byte, this call site needs a corresponding update to pass rawBody directly instead of wrapping it in the variadic form.
| if len(rawBody) > 0 { | |
| attachOpenAIResponsesRequestExtensions(chatReq, req, rawBody[0]) | |
| } | |
| if len(rawBody) > 0 { | |
| attachOpenAIResponsesRequestExtensions(chatReq, req, rawBody) | |
| } |
| func cloneRawMessage(src json.RawMessage) json.RawMessage { | ||
| if len(src) == 0 { | ||
| return nil | ||
| } | ||
|
|
||
| return append(json.RawMessage(nil), src...) | ||
| } |
There was a problem hiding this comment.
cloneRaw helper
cloneRawMessage here and cloneRaw in request_extensions.go do exactly the same thing. The duplication is harmless but could diverge over time. Consider exporting cloneRawMessage from the llm package (or keeping it unexported and referencing it from request_extensions.go via the llm package) to have a single source of truth.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
感谢 PR，感觉 |
修复 Responses 非透传模式下工具原语保真
背景
OpenAI Responses 非透传转换链路会将请求归一化为通用
llm.Request，部分 Provider 原生字段无法完整表达，例如tool_search、tool_choice.tools以及部分原始inputitem，导致出站请求丢失工具相关信息。修改内容
llm.Request增加ProviderExtensionssidecar，用于保存 Provider 私有原始片段，并通过json:"-"避免污染通用序列化。ProviderExtensions不参与 JSON 序列化。变更范围
本次修复仅覆盖 OpenAI Responses 请求侧的非透传转换链路，不涉及响应/SSE 透传、UI、GraphQL 或系统配置。
验证
已运行：
关联：#1525