Repository navigation
gateway: keep one Chat choice when translating, including relay indexes - #168
Conversation
Add test to ensure translation keeps first choice intact.
yetone
left a comment
yetone
left a comment
There was a problem hiding this comment.
@charan-rathore 感谢！思路是对的：流式和非流式翻译都走 chatDecoder，新测试在 main 上失败（BetaAlpha again done!），加上修复后通过；choice 1 先到、缺 index 或 index 为 null、只有 usage 的块、choice 0 里的并行工具调用、Chat→Chat 带 n:2 的透传，我都实际跑过，结果正确。不过有两种以前能用的中转站返回会被这个改动弄坏，合并前需要修一下：
- 总是发非 0 index 的中转站会丢掉整条回复（chat.go:391-395，
if c.Index != 0 { continue }）：上游只有一个 choice、但写成 "index":1 时，Anthropic 和 Responses 的回复都是 200 但 content/output 为空、usage 为 0，流式和非流式都一样；main 上会正常返回 "Hello"。magpie 从不发 n，所以这正是最可能出现的情况，而且是悄悄失败。 - index 是字符串时每个块都被丢掉（chat.go:365，
Index int）："index":"0" 会让 json.Unmarshal 报类型错误，decode 出错就返回 nil，整条回复为空；main 不解析这个字段，所以没问题。
建议：锁定第一个出现的 choice 的 index，而不是写死 0；index 宽松解析（比如 json.RawMessage，或者可空数字并忽略错误）。改好后我再拉下来验证。
Updated the chatDecoder struct to include choice as a string and modified the handling of the index in the decode method to support various formats.
Refactor comments for clarity and update test cases to ensure proper handling of choice indices in chat translation.
Refactor chat handling by removing unused types and functions. Introduce new functions for processing chat responses and tool calls.
Refactor chat handling to support new request structure and improve parsing logic.
charan-rathore
changed the title
|
Thanks for the update — both points from my last review are fixed; I checked through One thing left (
Suggested fix: a missing, null or unparseable index should neither set the lock nor be filtered — treat it as the current choice. Only a readable index locks or filters (and parse |
yetone
left a comment
yetone
left a comment
There was a problem hiding this comment.
See comment above: a missing/null index is coerced to 0 and can drop the whole reply.
Updated tool message handling to include images in user messages. Enhanced the tool result message format to include image URLs and additional notes about returned images.
|
Thanks for the careful retest. I reproduced all three cases on the previous branch, then changed the decoder so only a readable choice index selects or filters the reply. Missing, null and malformed indexes continue the current choice; 1.0 and 1 match. The new regression tests failed before the fix, and the full gateway suite passes after it. I've updated #168 for another look. |
|
Thanks! Re-reviewed the new head line by line and ran it end to end through the gateway against scripted upstreams — all the cases from my previous review now pass, and the new tests fail without the change. Merged; ships in v0.1.350. (#168 had been closed at the same moment as your update comment, which looked accidental, so I reopened it to merge.) |
A translated reply has room for one choice. When a provider streams several choices, magpie now keeps the first choice it sees rather than joining their text, tool calls and stop reasons.
Relays are messy about indexes, so the decoder is lenient: it locks onto the first readable choice index it sees (whether that is 0, 1, a string "0", or a decimal like 1.0), and a chunk with a missing, null or unreadable index simply continues the current choice instead of being dropped. Chat-to-Chat passthrough remains unchanged.
Tests cover interleaved choices, a single choice indexed 1, a string index, missing/null/malformed indexes, a decimal index, an unindexed choice after a usage-only chunk, and existing parallel tool calls. Tested with
GOPROXY=off GOSUMDB=off go test ./internal/gateway -count=1.This follows yetone's requested-changes reviews on #168. Instinct assisted with the change and tests.