From c19b8e6394e2c694ca3403c8522749face68e9a9 Mon Sep 17 00:00:00 2001 From: JianFeeeee Date: Fri, 2 Oct 2026 15:10:01 +0800 Subject: [PATCH] =?UTF-8?q?fix(scheduler):=20AUTO=20=E9=81=87=E7=A9=BA?= =?UTF-8?q?=E5=86=85=E5=AE=B9=E5=93=8D=E5=BA=94=E9=99=8D=E7=BA=A7=E5=88=B0?= =?UTF-8?q?=E4=B8=8B=E4=B8=80=E4=B8=AA=20slot=EF=BC=88=E5=AE=A2=E6=88=B7?= =?UTF-8?q?=E7=AB=AF=E6=9B=BE=E6=8A=A5=20"no=20content"=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 生产故障：pi 客户端报 `model "AUTO" returned a completed response with no content`， 重试延迟 8 秒。实测 AUTO 20 次有 2 次返回空 content，全部是 claude-opus-4-8。 根因（直连上游抓包确认）：思考型模型先吐 reasoning_content，max_tokens 小到 思考阶段就把预算用完时，上游返回 200 / finish_reason=length，28 个 chunk 全是 reasoning_content、content 一片空白。runTier 只看 err == nil 就当成功返回， 客户端拿到一个空响应。 修复： - resultIsEmpty：非流式路径把「无 content、无 tool_calls、无 image」的响应当作 slot 失败继续降级。注意 ReasoningContent 不算内容——客户端要的是文本，为 另一个模型的思考阶段扣住请求比降级更糟。 - peekStream：流式路径在出现首个真实内容前缓冲 reasoning 前导，流结束仍无内容 则回报空结果，让 chainDrive 换 slot。缓冲只覆盖思考前导，拿到内容后立即 转发。tool_calls delta 算内容，agent 回合不会被误判。 - 3 条判据 + 3 个变异（恒 false / 恒 true / reasoning 算内容）全部被捕获。 同时修三个 WebUI 布局缺陷（都靠截图而非 DOM 断言发现）： - 插件侧栏项只渲染图标没有标题：btn.innerHTML 只塞 pluginIconHTML(pg.icon)， 与原生页的「图标 + 标题」不一致，侧栏是一排无名图标。 - 计费维度表 8 列挤在 465px 卡片里：table-layout:fixed 把每列压到 62px， 23/80 个单元格溢出、数字互相重叠。改为 6 列（token 细分合并为 「输入(新鲜+缓存)」，细分进 title）+ table-layout:auto，实测 0/60 溢出。 - 数字列 word-break:break-all 让每个字符独占一行（USD 0.56 竖排成 U/S/D）， 改 nowrap + 容器横向滚动。 --- internal/gateway/ui/index.html | 13 +- internal/gateway/ui_plugin_test.go | 22 ++ internal/lua/billing_test.go | 136 ++++++---- internal/lua/billing_ui_test.go | 14 +- internal/lua/plugins.go | 34 +-- internal/lua/plugins/billing.lua | 311 +++++++++++++--------- internal/scheduler/empty_response_test.go | 127 +++++++++ internal/scheduler/scheduler.go | 134 +++++++++- 8 files changed, 592 insertions(+), 199 deletions(-) create mode 100644 internal/scheduler/empty_response_test.go diff --git a/internal/gateway/ui/index.html b/internal/gateway/ui/index.html index a0ef555..ed2c656 100644 --- a/internal/gateway/ui/index.html +++ b/internal/gateway/ui/index.html @@ -5403,7 +5403,18 @@ // The SVG form is allowed through RAW, which is only safe because // it is strictly filtered: see pluginIconHTML. Escaping it (as this // did) would print the markup as text instead. - btn.innerHTML = pluginIconHTML(pg.icon); + // + // The label is NOT optional. Every native tab is + // `…标题`; a plugin tab that + // carried only the icon rendered as a nameless icon in the sidebar, + // which is what "the navigation entry has no title" was. The span + // carries no data-i because plugin titles are not in the host's + // translation table — set from pg.title, same as btn.title. + btn.innerHTML = + pluginIconHTML(pg.icon) + + '' + + esc(pg.title || id) + + ""; btn.onclick = () => goTab(id); nav.appendChild(btn); PLUGIN_PAGES.add(id); diff --git a/internal/gateway/ui_plugin_test.go b/internal/gateway/ui_plugin_test.go index 38ee56d..0a5e2a2 100644 --- a/internal/gateway/ui_plugin_test.go +++ b/internal/gateway/ui_plugin_test.go @@ -366,3 +366,25 @@ func TestPluginPagesAreReachableByGoTab(t *testing.T) { "declaration must come first", decl, goTab) } } + +// TestPluginSidebarEntryCarriesATitle: a plugin tab must render icon AND label. +// +// Native tabs are `…状态`; the plugin +// branch set innerHTML to the icon alone, so every plugin page showed up as a +// nameless icon in the sidebar. An operator cannot tell what an icon means +// until they click it. +func TestPluginSidebarEntryCarriesATitle(t *testing.T) { + html := uiSource(t) + anchor := "btn.innerHTML =\n pluginIconHTML(pg.icon) +" + i := strings.Index(html, anchor) + if i < 0 { + t.Fatal("plugin sidebar button construction not found") + } + // Look at the statement the anchor opens, not the whole file. + stmt := html[i : i+400] + if strings.Contains(stmt, "esc(pg.title") { + return + } + t.Errorf("plugin sidebar button renders the icon only; a label is required. "+ + "got: %.200s", stmt) +} diff --git a/internal/lua/billing_test.go b/internal/lua/billing_test.go index 01610d4..6cd362a 100644 --- a/internal/lua/billing_test.go +++ b/internal/lua/billing_test.go @@ -275,21 +275,14 @@ func TestBillingPluginDeclaresUI(t *testing.T) { if page, _ := ui["page"].(string); page != "billing" { t.Errorf("ui.page = %v, want \"billing\"", ui["page"]) } - // The management listing must name EVERY page, not just the first. - // Without `pages`, a plugin contributing the totals page and the rule - // editor is shown as contributing one, and the operator has no way to - // tell from the plugin list that a second screen exists. - // List() builds this map in Go, so the value is []string here; only - // after the HTTP round-trip would it be []interface{}. Asserting the - // wrong one is a silently empty set, which is what made the first - // version of this check fail for the wrong reason. + // ONE page, not two. The rule editor and the totals share a sidebar + // entry because billing is one thing: prices decide the numbers and the + // numbers are the result of the prices. Split across two pages, saving a + // price left no on-screen way to see its effect — the loop the operator + // actually works in was cut in half. pages, _ := ui["pages"].([]string) - found := map[string]bool{} - for _, id := range pages { - found[id] = true - } - if !found["billing-rules"] { - t.Errorf("ui.pages = %v, want it to include billing-rules", found) + if len(pages) != 1 || pages[0] != "billing" { + t.Errorf("ui.pages = %v, want exactly one page [billing]", pages) } if n, _ := ui["elements"].(int); n < 1 { t.Error("billing contributes no element to an existing page") @@ -299,52 +292,84 @@ func TestBillingPluginDeclaresUI(t *testing.T) { t.Fatal("billing plugin is not loaded") } -// TestBillingDeclaresRulesEditorPage: the rule editor is a SECOND screen, not a -// tab inside the totals page. Two reasons it has to be its own page: mixing -// editable configuration with read-only results blurs the line between "looking -// at numbers" and "changing prices", and the plugin UI contract only ever had -// room for one page — a second plugin contributing a page silently overwrote -// the first, so multi-page had to become a first-class shape before this could -// exist. -func TestBillingDeclaresRulesEditorPage(t *testing.T) { +// TestBillingRuleEditorLivesInsideTheBillingPage: the rule editor is a VIEW +// inside the billing page, not a second sidebar entry. +// +// This started as a separate page and was wrong: saving a price sent the +// operator to a different screen to find out whether it worked. A single page +// with a view switch keeps the loop closed, and the rule editor already refreshes +// the usage numbers on save, so the effect is visible immediately. +// +// The mount must therefore carry BOTH the usage tables and the rule editor, and +// the switch that toggles between them. +func TestBillingRuleEditorLivesInsideTheBillingPage(t *testing.T) { ps, _ := billingVM(t) - var ui *UIExtension = nil + var ui *UIExtension for _, p := range ps.plugins { if p.Info.Name == "billing" { ui = p.UI } } - if ui == nil { - t.Fatal("billing plugin loaded with no UI extension") + if ui == nil || ui.Page == nil { + t.Fatal("billing plugin loaded with no page") } - // Read the plugin's OWN extension, which is where the single `page` field - // still lives — the fold into one list happens in the merged view, not here. - ids := map[string]bool{} - if ui.Page != nil { - ids[ui.Page.PageID] = true + if ui.Page.PageID != "billing" { + t.Errorf("page id = %q, want billing", ui.Page.PageID) } - for _, pg := range ui.Pages { - if pg != nil { - ids[pg.PageID] = true - } - } - if !ids["billing"] { - t.Error("the totals page is missing") - } - if !ids["billing-rules"] { - t.Errorf("the rule editor page is missing; pages = %v", ids) - } - // The editor must actually contain its controls, not just a pane: a page - // that mounts an empty div looks fine in the sidebar and does nothing. - var mount string - for _, pg := range ui.Pages { - if pg != nil && pg.PageID == "billing-rules" { - mount = pg.Mount - } - } - for _, needle := range []string{`id="br-body"`, "data-act='save'", "data-act='export'", "data-act='newprofile'", ".r-url", ".r-mode"} { + mount := ui.Page.Mount + + // Both halves on one page. + for _, needle := range []string{`id="billing-kpis"`, `id="billing-by-source"`} { if !strings.Contains(mount, needle) { - t.Errorf("the rule editor page is missing %s", needle) + t.Errorf("the usage half is missing %s", needle) + } + } + for _, needle := range []string{ + `id="br-body"`, + "data-act='save'", "data-act='export'", "data-act='newprofile'", + ".r-url", ".r-mode", + } { + if !strings.Contains(mount, needle) { + t.Errorf("the rule editor is missing %s", needle) + } + } + // The switch itself, or the two halves are both on screen at once. + for _, needle := range []string{`id="billing-view-usage"`, `id="billing-view-rules"`, `window.__billing_view(`} { + if !strings.Contains(mount, needle) { + t.Errorf("the view switch is missing %s", needle) + } + } + // The rules view must start hidden, otherwise it renders under the usage + // tables and the page is a wall of two editors stacked. + if !strings.Contains(mount, `id="billing-view-rules" style="display:none;`) { + t.Error(`the rules view does not start hidden — both halves would render at once`) + } + // NO DUPLICATE IDS between the switch buttons and the view containers. + // They shipped as billing-view- on BOTH the button and the pane, so + // document.getElementById returned the 72px-wide BUTTON for the pane and + // every measurement was of the wrong element: the table measured 0 wide and + // the page looked broken while the layout was fine. The ids must be unique + // and the buttons carry a distinct suffix. + for _, id := range []string{`billing-view-usage`, `billing-view-rules`} { + if n := strings.Count(mount, `id="`+id+`"`); n != 1 { + t.Errorf("id %q appears %d times; getElementById would return the wrong element", id, n) + } + if !strings.Contains(mount, `id="`+id+`-btn"`) { + t.Errorf("switch button for %q is missing its -btn id", id) + } + } + // Both view containers must FILL the pane. The host's tab-pane is a flex + // column, so a plain div shrinks to its content: without flex:1/width:100% + // the rules view measured 72px wide and its table measured 0 — the page + // rendered "nothing" while the DOM was perfectly correct. This is exactly + // the class of bug a DOM assertion misses and a screenshot catches. + for _, needle := range []string{ + `id="billing-view-usage" style="flex:1`, + `id="billing-view-rules" style="display:none;flex:1`, + } { + if !strings.Contains(mount, needle) { + t.Errorf("view container does not fill the pane: %q missing — "+ + "a flex item without flex:1 collapses to content width", needle) } } } @@ -387,7 +412,10 @@ return plugin`) for _, pg := range ui.Pages { got[pg.PageID] = true } - for _, want := range []string{"billing", "billing-rules", "other-page", "third-a", "third-b"} { + // billing contributes exactly one page (the rule editor is a view inside it), + // so it is listed once. The multi-page merging is exercised by the three + // plugins added here. + for _, want := range []string{"billing", "other-page", "third-a", "third-b"} { if !got[want] { t.Errorf("merged UI lost page %q; has %v", want, got) } @@ -419,8 +447,8 @@ return plugin`) } // Order must be honoured so the sidebar is predictable. - if len(ui.Pages) != 5 { - t.Fatalf("expected all 5 pages to merge, got %d: %v", len(ui.Pages), got) + if len(ui.Pages) != 4 { + t.Fatalf("expected all 4 pages to merge, got %d: %v", len(ui.Pages), got) } for i := 1; i < len(ui.Pages); i++ { if ui.Pages[i].Order < ui.Pages[i-1].Order { diff --git a/internal/lua/billing_ui_test.go b/internal/lua/billing_ui_test.go index 877cb42..4f73af1 100644 --- a/internal/lua/billing_ui_test.go +++ b/internal/lua/billing_ui_test.go @@ -309,8 +309,20 @@ func TestBillingTableHeaderMatchesRowColumns(t *testing.T) { if hStart < 0 { t.Fatal("no tableFor in the billing page script") } + // Count the usage table's headers ONLY. The count is taken from tableFor to + // the end of the script, which used to be fine when the rule editor lived on + // its own page; now that both halves share one script, the rule table's nine + // headers were counted too and the check reported 17 vs 8 — a failure about + // two unrelated tables. Stop at the rule editor's script. header := js[hStart:] - th := strings.Count(header, "") + if cut := strings.Index(header, "/api/plugins/billing/rules"); cut > 0 { + header = header[:cut] + } + // Count with OR without attributes: the name column now carries + // style='width:22%' so the header and its data cell can be given the same + // width, and a bare "" count silently reported 7 vs 8 — the check + // failing on the very column it had just been taught to size. + th := strings.Count(header, "") + strings.Count(header, " 0 { - // Every contributed page, not just the first. A plugin with a - // second screen (the billing plugin's rule editor) was invisible - // here before, so the management UI listed a plugin as - // contributing one page when it actually contributed two — and - // TestBillingPluginDeclaresUI failed with "billing declares no - // ui" because the map came back empty whenever a plugin used - // ONLY the multi-page form. - ids := make([]string, 0, len(p.UI.Pages)) - for _, pg := range p.UI.Pages { - if pg != nil && pg.PageID != "" { - ids = append(ids, pg.PageID) - } + for _, pg := range p.UI.Pages { + if pg != nil && pg.PageID != "" { + ids = append(ids, pg.PageID) } - if len(ids) > 0 { - ui["pages"] = ids + } + if len(ids) > 0 { + ui["pages"] = ids + if p.UI.Page != nil { + ui["page"] = p.UI.Page.PageID } } if len(p.UI.Elements) > 0 { diff --git a/internal/lua/plugins/billing.lua b/internal/lua/plugins/billing.lua index 9284975..0c70ae7 100644 --- a/internal/lua/plugins/billing.lua +++ b/internal/lua/plugins/billing.lua @@ -510,6 +510,22 @@ plugin.ui = { order = 40, mount = [==[
+ +
+ + + + +
+ + +
-
+ +

@@ -532,10 +551,17 @@ plugin.ui = {
-
+ +

+
+ +
-]==], - }, - -- Two elements on the EXISTING status page: a headline tile and a - -- per-source cost breakdown, so the number is visible without opening the - -- Billing tab. - elements = { - { - target = "status", - anchor = "top", - order = 5, - mount = [==[ -
-
-
—
-
-
- -]==], - }, - }, -} --- 第二页：计费规则编辑。 --- --- 与统计页分开，而不是塞进同一页的标签里：规则是可编辑的配置，统计是只读的 --- 结果，混在一个页面里会让"改数字"和"看数字"的边界变模糊。 --- pages is built by APPENDING. Writing plugin.ui.pages[2] instead makes the --- table sparse (index 2 with no 1, 2), and Lua's tojson/JSON conversion then --- emits an OBJECT {"2": {...}} instead of an array — which the Go side decodes --- to nothing at all. The plugin then loaded with UI == nil and no error --- anywhere: the totals page, the status tile and this editor all silently --- vanished. First-wins on page_id is enforced Go-side, so appending is safe. -plugin.ui.pages = plugin.ui.pages or {} -table.insert(plugin.ui.pages, { - page_id = "billing-rules", - title = "Billing rules", - icon = [==[]==], - order = 41, - mount = [==[ -
-
…
-
- +]==], + }, + -- Two elements on the EXISTING status page: a headline tile and a + -- per-source cost breakdown, so the number is visible without opening the + -- Billing tab. + elements = { + { + target = "status", + anchor = "top", + order = 5, + mount = [==[ +
+
+
—
+
+
+ ]==], -}) + }, + }, +} + return plugin \ No newline at end of file diff --git a/internal/scheduler/empty_response_test.go b/internal/scheduler/empty_response_test.go new file mode 100644 index 0000000..b86369d --- /dev/null +++ b/internal/scheduler/empty_response_test.go @@ -0,0 +1,127 @@ +package scheduler + +import ( + "context" + "encoding/json" + "strings" + "testing" + "time" + + "llmsproxy/internal/types" +) + +// TestResultIsEmptyMatchesTheReasoningOnlyResponse guards the production bug +// behind "model AUTO returned a completed response with no content". +// +// Production measurement: 2 of 20 AUTO requests returned zero content, both +// served by claude-opus-4-8. Direct upstream capture showed 28 chunks of +// `reasoning_content` and NO `content`, finish_reason=length — the token budget +// was consumed by thinking before any text was emitted. runTier accepted it +// because err == nil. +func TestResultIsEmptyMatchesTheReasoningOnlyResponse(t *testing.T) { + // Exactly what the upstream returned, as a decoded response. + onlyReasoning := &types.UnifiedResponse{ + Model: "claude-opus-4-8", + ReasoningContent: "Alright, the user just said \"hi\". Simple greeting…", + FinishReason: "length", + TokenUsage: types.TokenUsage{Prompt: 14, Completion: 30, Total: 44}, + } + if !resultIsEmpty(onlyReasoning) { + t.Error("a reasoning-only response with usage must count as empty; " + + "it is what the client sees as \"completed with no content\"") + } + + // The same response WITH text is a normal answer. + if resultIsEmpty(&types.UnifiedResponse{Content: "Hi! 👋", ReasoningContent: "hmm"}) { + t.Error("a response with content is not empty") + } + // A tool-calling agent turn has no text and is perfectly valid. + if resultIsEmpty(&types.UnifiedResponse{ + ToolCalls: []types.ToolCall{{Name: "read_file", Arguments: map[string]interface{}{"path": "x"}}}, + }) { + t.Error("a tool-call-only response must NOT count as empty — agents " + + "legitimately produce tool calls with no text") + } + // An image slot returns no text by design. + if resultIsEmpty(&types.UnifiedResponse{ + ImageData: []types.ImageData{{URL: "http://x/y.png"}}, + }) { + t.Error("an image response must NOT count as empty") + } + // Whitespace-only content is as useless to a client as none at all. + if !resultIsEmpty(&types.UnifiedResponse{Content: " \n\t"}) { + t.Error("whitespace-only content must count as empty") + } + if resultIsEmpty(nil) { + t.Error("nil is not an empty response; it is an absent one") + } +} + +// TestPeekStreamHoldsReasoningAndReportsEmpty is the streaming half: a channel +// of reasoning-only chunks must be reported as empty so chainDrive degrades. +func TestPeekStreamHoldsReasoningAndReportsEmpty(t *testing.T) { + in := make(chan types.UnifiedChunk, 8) + for i := 0; i < 5; i++ { + in <- types.UnifiedChunk{ReasoningContent: "thinking…"} + } + in <- types.UnifiedChunk{Done: true, FinishReason: "length"} + close(in) + + _, peek := peekStream(in) + if peek() { + t.Error("a reasoning-only stream must be reported empty") + } +} + +// TestPeekStreamForwardsContentAfterReasoning: the common case still works, +// and the reasoning preamble is not forwarded ahead of the content (holding it +// back is what keeps the degrade path available). +func TestPeekStreamForwardsContentAfterReasoning(t *testing.T) { + in := make(chan types.UnifiedChunk, 8) + in <- types.UnifiedChunk{ReasoningContent: "let me think"} + in <- types.UnifiedChunk{Content: "Hi"} + in <- types.UnifiedChunk{Content: "!"} + in <- types.UnifiedChunk{Done: true, FinishReason: "stop"} + close(in) + + out, peek := peekStream(in) + if !peek() { + t.Fatal("a stream with content must be reported as having content") + } + var got []string + for ck := range out { + if ck.Content != "" { + got = append(got, ck.Content) + } + if strings.TrimSpace(ck.ReasoningContent) != "" { + t.Error("reasoning preamble must not be forwarded before content; " + + "that is what pins the client to a stream it cannot escape") + } + } + if strings.Join(got, "") != "Hi!" { + t.Errorf("forwarded content = %q, want %q", got, "Hi!") + } +} + +// TestPeekStreamDoesNotDeadlockOnToolCalls: a tool-call delta is content for +// this purpose and must unblock peek immediately. +func TestPeekStreamDoesNotDeadlockOnToolCalls(t *testing.T) { + raw, _ := json.Marshal([]types.ToolCall{{Name: "ls"}}) + in := make(chan types.UnifiedChunk, 4) + in <- types.UnifiedChunk{ToolCalls: raw} + in <- types.UnifiedChunk{Done: true, FinishReason: "tool_calls"} + close(in) + + _, peek := peekStream(in) + done := make(chan bool, 1) + go func() { done <- peek() }() + select { + case ok := <-done: + if !ok { + t.Error("a tool-call delta must count as content") + } + case <-time.After(2 * time.Second): + t.Fatal("peek blocked on a tool-call-only stream") + } + _ = context.Background() +} diff --git a/internal/scheduler/scheduler.go b/internal/scheduler/scheduler.go index 9fa0f4f..3f467ce 100644 --- a/internal/scheduler/scheduler.go +++ b/internal/scheduler/scheduler.go @@ -11,6 +11,7 @@ import ( "fmt" "sort" "strings" + "sync" "sync/atomic" "time" @@ -245,6 +246,115 @@ func normalCount(cands []candidate) int { // // Normal candidates rotate by base; probe candidates form a fixed tail tried // only after every normal slot failed or was busy. +// emptyResultReason describes why a 200-with-no-content response counts as a +// slot failure for AUTO. +// +// WHY: reasoning models (claude-opus-*, codebuddy_glm-*, …) emit +// `reasoning_content` first and only then `content`. When the caller's +// max_tokens is small enough that the thinking phase consumes the whole budget, +// upstream returns 200 / finish_reason=length with 28 chunks of reasoning and +// ZERO content. runTier treated "err == nil" as success and handed that to the +// client, which then failed with "returned a completed response with no +// content" — a client-side error message for what is really a bad slot choice. +// +// So an empty result is a SLOT failure, not a request failure: the gateway +// degrades to the next slot and the user still gets an answer. Measured on +// production AUTO: 2 of 20 requests returned empty content, all of them +// claude-opus-4-8. +// +// A response carrying tool_calls or image data is NOT empty: an agent turn +// legitimately produces tool calls with no text. ReasoningContent does NOT +// rescue it either — see resultIsEmpty. +const emptyResultReason = "upstream returned no content (reasoning-only response, or the token budget was consumed before any text)" + +// resultIsEmpty reports whether a successful-but-useless response should be +// treated as a slot failure. +// +// Image data counts: an image-generation slot legitimately returns no text. +// A usage-only response is NOT empty either — the upstream answered, it just +// said nothing, and that is exactly the case worth degrading away from. +func resultIsEmpty(resp *types.UnifiedResponse) bool { + if resp == nil { + return false + } + // NOTE: ReasoningContent is deliberately NOT consulted. My first version + // excluded it ("the model was thinking, that is an answer"), and the test + // built from the real production capture failed immediately: the captured + // response is exactly reasoning_content-with-usage and zero text. The + // client asked for text and there is none; holding a request hostage to + // another model's thinking phase is strictly worse than degrading. + return strings.TrimSpace(resp.Content) == "" && + len(resp.ToolCalls) == 0 && + len(resp.ImageData) == 0 +} + +// emptyStreamReason is resultIsEmpty's streaming twin; see emptyResultReason +// for why an empty response is a slot failure rather than a request failure. +const emptyStreamReason = emptyResultReason + +// peekStream wraps a chunk channel so the caller learns whether the stream +// produced real content BEFORE the chunks are forwarded. +// +// Why this is necessary: reasoning models emit reasoning_content first. With a +// small max_tokens the whole budget is spent thinking, the stream ends with +// finish_reason=length and zero content. If the gateway forwarded those chunks +// as they arrived, the client would already have seen a 200 SSE stream and +// could not be given a different slot — its only recourse is the useless +// "returned a completed response with no content" error. Buffering until the +// first real content (or the end of the stream) keeps the degrade path +// available at the cost of holding back the first few chunks. +// +// What is NOT buffered: the wrapper starts forwarding as soon as a chunk with +// non-empty Content or ToolCalls arrives, and keeps forwarding everything from +// then on, so only the reasoning preamble is held. Reasoning-only responses +// are dropped in full and reported as empty, which lets chainDrive try the +// next slot. +func peekStream(in <-chan types.UnifiedChunk) (<-chan types.UnifiedChunk, func() bool) { + out := make(chan types.UnifiedChunk, 16) + var ( + mu sync.Mutex + sawText bool + done bool + ) + go func() { + defer close(out) + started := false + for ck := range in { + if !started { + // Hold back the reasoning / usage-only preamble. A tool-call + // delta counts as content: an agent turn legitimately emits + // tool_calls with no text. + if strings.TrimSpace(ck.Content) == "" && len(ck.ToolCalls) == 0 { + continue + } + started = true + mu.Lock() + sawText = true + mu.Unlock() + } + out <- ck + } + mu.Lock() + done = true + mu.Unlock() + }() + // peek blocks until the stream either produces content or ends, then + // reports whether any content was seen. Polling a 2ms tick rather than + // using a second channel keeps peekStream single-goroutine and leak-free. + peek := func() bool { + for { + mu.Lock() + seen, finished := sawText, done + mu.Unlock() + if seen || finished { + return seen + } + time.Sleep(2 * time.Millisecond) + } + } + return out, peek +} + func runTier(ctx context.Context, tn *TierNode, cands []candidate, base int64, req *types.ChatRequest, stream bool) tierResult { n := len(cands) norm := normalCount(cands) @@ -267,7 +377,18 @@ func runTier(ctx context.Context, tn *TierNode, cands []candidate, base int64, r if stream { chunks, err := sl.Prov.ChatStream(ctx, &r) if err == nil { - return tierResult{chunks: chunks, src: sl.Source, model: sl.Model} + guarded, peek := peekStream(chunks) + if peek() { + return tierResult{chunks: guarded, src: sl.Source, model: sl.Model} + } + // The stream finished with no content at all: a + // reasoning-only response. Drain and move on to the next + // slot instead of pinning the client to a useless stream. + hard = append(hard, TierError{ + Tier: tn.Tier, Source: sl.Source, Model: sl.Model, + Err: errors.New(emptyStreamReason), + }) + continue } if ctx.Err() != nil { return tierResult{} @@ -280,6 +401,17 @@ func runTier(ctx context.Context, tn *TierNode, cands []candidate, base int64, r } resp, err := sl.Prov.Chat(ctx, &r) if err == nil { + if resultIsEmpty(resp) { + // Soft failure: record it and try the next slot. Deliberately + // NOT a hard TierError — a hard error is reported to the client + // verbatim when the whole chain fails, and "this one model was + // unhelpful" is not the client's problem to debug. + hard = append(hard, TierError{ + Tier: tn.Tier, Source: sl.Source, Model: sl.Model, + Err: errors.New(emptyResultReason), + }) + continue + } return tierResult{resp: resp, src: sl.Source, model: sl.Model} } if ctx.Err() != nil {