Repository navigation
fix(translator): always emit client-required fields on the Responses wire - #77
Conversation
…wire The Responses client (async-openai fork 95b52eb) deserializes ResponseUsage, OutputTextContent, ReasoningItem, SummaryTextContent and FunctionToolCall without serde defaults: an absent field fails the whole turn with `missing field `x``. - usage: always emit input_tokens_details / output_tokens_details (upstream Chat usage often omits both; the conditional spread used to drop them -> `missing field input_tokens_details`) - output_text parts: always emit annotations: [] - reasoning items: emit summary: [] on open; summary parts carry text: "" - function_call items: emit arguments: "" on added (deltas stream after) Adds a "client-required wire fields" test block pinning the contract, plus a usage regression test for detail-less upstream Chat usage.
|
收到,我看下情况。之前这个responses系列接线是codex顺手做的 |
TriDefender
left a comment
There was a problem hiding this comment.
The reported incompatibility is real: the pinned async-openai fork requires the affected fields, and the PR adds them to the relevant batch and streaming paths. The full Bun suite passed locally (1,108 tests), but the required TypeScript check fails, so this is not ready to merge.
Please also strengthen the streaming tests to assert that each expected added/done event exists before checking its payload; the current loops can pass without inspecting an event. Cover the terminal response output, namespaced function calls, and preservation of nonzero usage details. Consider requiring the inner usage counters in the TypeScript type and using an output-specific type or helper for text parts.
This assessment is based on the pinned client schema, translator paths, and tests; I did not run Grok Build end to end.
|
谢谢 review,逐条对账:
另外:PR 描述里提到的 wire-audit 审计脚本如需要,我可以发 gist,方便你端到端复验。修完这些会再请求 re-review。 |
|
麻烦提交前跑完一轮test |
Address review on PR TriDefender#77: - Fix all 5 tsc errors in chat-to-responses.test.ts: declare `annotations` in the asserted content-part shape, route union casts through `unknown`, add the missing `created` field to the batch fixture. - Stream tests now assert target events EXIST before inspecting payloads (the old filter-loops passed vacuously when the event under test was missing), and also cover the terminal `response.output` fields, the namespaced function_call branch, and preservation of non-zero cached_tokens / reasoning_tokens from upstream usage. - Tighten types: ResponsesUsage detail counts (cached_tokens / reasoning_tokens) are now required, matching the wire; output-text annotations moved from the shared ResponsesContentPart to a dedicated ResponsesOutputTextPart with required `text` + `annotations`. - Document the "0 means unavailable as well as zero" semantics of the usage fallback in chatUsageToResponsesUsage. Full suite: bun test 1109 pass / 0 fail; bun x tsc --noEmit clean.
|
跑完一轮了,全部绿:
顺带把这轮 review 里答应对齐的都做掉了(
|
The dedicated output-text part type introduced for the tightened annotations contract is now used by the message variant of ResponsesOutputItem, instead of widening through ResponsesContentPart.
|
补两个小提交:
|
|
glm 5.3 flash是傻逼,请不要管它说的有的没的(流汗黄豆 |
TriDefender
left a comment
There was a problem hiding this comment.
复审了 9327725:上次提出的类型检查错误已经修复;流式事件、终态 output、命名空间函数调用和非零用量的覆盖已补充;usage 缺失时补 0 的取舍也已在 PR 描述中说明。独立执行 bun x tsc --noEmit 通过,bun test 为 1109 pass / 0 fail。此前请求修改的问题已解决,没有发现新的阻塞问题。
下面有一处非阻塞的注释措辞建议。此次复审没有实际运行 Grok Build 或重复上游实弹审计。
| // SEMANTICS OF 0: an emitted `cached_tokens: 0` / `reasoning_tokens: 0` | ||
| // means "upstream did not report this breakdown", NOT a measured zero. | ||
| // The Anthropic translation layer only reports cache buckets when the | ||
| // upstream sends them and never reports reasoning tokens, so responses |
There was a problem hiding this comment.
非阻塞:这里和 ResponsesUsage 的注释写成“0 表示未上报,而不是实测为 0”过于绝对。代码也会原样保留上游明确上报的 0;建议与 PR 描述一致,写成“0 可能表示未上报,也可能是上游报告的 0,无法区分”。
fix(translator): always emit client-required fields on the Responses wire
问题
/v1/responses路径的翻译器(chat-to-responses)在若干响应结构上把字段写成条件发射或空壳。部分严格客户端(本例为 Grok Build 内嵌的 async-openai fork95b52eb)对这些结构做无 serde default 的反序列化——任何一个字段缺席,整轮请求直接以missing field \x`` 失败:ResponseUsageinput_tokens_details/output_tokens_detailsOutputTextContentannotationsReasoningItemsummarySummaryTextContenttextFunctionToolCallarguments(added事件)对宽容客户端(官方 OpenAI SDK 等)无影响——官方服务端总会带齐这些字段,所以此前未暴露。
修复
input_tokens_details/output_tokens_details,上游缺省时补cached_tokens: 0/reasoning_tokens: 0(去掉原来的条件展开)。语义说明:这里的0兼指「上游未上报该分项」与「实测为 0」——Anthropic 转换链不上报 reasoning 计数,含 reasoning 的响应会显示reasoning_tokens: 0;这是「保住已知三个总量」与「不发明计数」之间的取舍,对消费 usage 数字的客户端,缺失→0 本身就是语义变化(见chatUsageToResponsesUsage注释)。annotations: []。output_item.added时发射summary: [];summary part 打开时带text: ""。output_item.added时发射arguments: "",实际参数随后续 delta 事件到达。类型收紧:
responses-types.ts中ResponsesUsage两个 details 对象及其内层cached_tokens/reasoning_tokens改为必填;annotations从共享的ResponsesContentPart移出,新增ResponsesOutputTextPart(text与annotations必填)只用于 output 侧,与实际 wire 契约一致。测试
client-required wire fields测试块(stream + batch)逐结构钉住客户端契约,并先断言目标事件存在再检查 payload——此前过滤式循环在目标事件缺失时会空过,测不出本要钉住的回归。response.completed终态response.output字段、namespaced function_call 分支、流式下非零cached_tokens/reasoning_tokens的保留。bun x tsc --noEmit干净(CI 上轮挂的 5 处类型错误已修复,推送前本地已加跑)。复现 / 验证方式
bun test src/translator/chat-to-responses.test.ts && bun x tsc --noEmit— 新增用例 + 类型检查。/v1/responses发送 stream=true 文本请求、stream=true 工具调用请求、非流式请求各一,对每个 SSE 事件检查:response.usage五字段齐、output_text部件带text+annotations、reasoning item 带summary[]、function_call item 带arguments。任一缺失即复现本 bug。备注
wire-audit.ts,约 200 行)未包含在本 PR 中,如需要可以补充或另开 gist。