-
Notifications
You must be signed in to change notification settings - Fork 616
fix(gui,combos): restore default effort picker and catalog combos #1092
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
53bcc3a
098039a
a89b409
0f9c7a0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,6 +63,7 @@ namespace, and cannot use reserved bare native families such as `gpt-*`, `o1-*`, | |
| | `strategy?` | `"failover" \| "round-robin"` | `"failover"` | Selection strategy. Target order is failover priority; weights shape smooth weighted round-robin. | | ||
| | `stickyLimit?` | `number` | `1` | Successful requests retained in one round-robin batch. Range 1–100. | | ||
| | `defaultEffort?` | `"low" \| "medium" \| "high" \| "xhigh" \| "max" \| "ultra" \| null` | unset | Applied only when the caller omits effort and the selected target advertises the requested rung. | | ||
| | `imageInput?` | `"auto" \| "disabled"` | `"auto"` | `"disabled"` drops image from the published modalities and rejects image-bearing requests before dispatch. Cannot enable image when a target lacks it. | | ||
| | `alias?` | `string` | — | Optional public model id in place of the canonical picker slug. | | ||
|
|
||
| ```json | ||
|
|
@@ -182,13 +183,16 @@ Per-request route-decision traces are recorded when a policy profile executes. | |
| A combo remains directly routable even when it cannot be listed. `ocx sync`, `/v1/models`, and the | ||
| Codex picker list it only when every target exposes capabilities that can be intersected: | ||
|
|
||
| - a positive `contextWindow`, from live metadata, registry hints, or provider | ||
| `modelContextWindows` / `contextWindow`; and | ||
| - a positive `contextWindow`, from live metadata, registry hints, provider | ||
| `modelContextWindows` / `contextWindow`, or — when the provider is known but every source | ||
| omits a window — a conservative 128,000-token fallback (clamped by `providerContextCaps` when | ||
| set); and | ||
|
Comment on lines
+186
to
+189
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This English fallback now allows known-provider targets without context metadata to catalog at 128k, but localized routing references still state that a positive AGENTS.md reference: docs-site/AGENTS.md:L9-L10 Useful? React with 👍 / 👎. |
||
| - a non-empty `inputModalities` intersection, treating an omitted member value as `["text"]`. | ||
|
|
||
| A bare relay id with no context metadata or targets with disjoint modalities removes the combo from | ||
| the catalog. Sync emits a summary warning and the dashboard marks it **Needs attention**. Add context | ||
| metadata, align modalities, or target models with discoverable compatible capabilities. | ||
| A target on an unknown/disabled provider with no discovery row, or targets with disjoint modalities, | ||
| removes the combo from the catalog. Sync emits a summary warning and the dashboard marks it | ||
| **Needs attention**. Add context metadata, align modalities, or target models with discoverable | ||
| compatible capabilities. | ||
|
|
||
| ## Request history and routing analytics | ||
|
|
||
|
|
@@ -222,3 +226,4 @@ The history index is disposable - deleting `routing-history.sqlite` triggers | |
| an automatic rebuild from `usage.jsonl` on the next query; `ocx logs | ||
| rebuild-index` forces one. Nothing in this system auto-tunes weights, | ||
| budgets, or candidate sets. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,6 +57,7 @@ selector 校验、冲突规则和隐私说明见[提供方配置](/reference/con | |
| | `strategy?` | `"failover" \| "round-robin"` | `"failover"` | 选择策略。目标顺序表示故障切换优先级;权重会影响平滑加权轮询。 | | ||
| | `stickyLimit?` | `number` | `1` | 在单个轮询批次中保留的成功请求数。范围 1–100。 | | ||
| | `defaultEffort?` | `"low" \| "medium" \| "high" \| "xhigh" \| "max" \| "ultra" \| null` | unset | 仅在调用方省略 effort 且所选目标声明了请求的档位时应用。 | | ||
| | `imageInput?` | `"auto" \| "disabled"` | `"auto"` | `"disabled"` 会从对外能力中去掉图片,并在分发前拒绝带图请求;不能在目标不支持时强开图片。 | | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Update the Chinese catalog-eligibility text. Lines 86-92 still state that a target without context metadata removes the combo from the catalog. As per path instructions, translated locale pages must not contradict the English source. Based on learnings, flag localized content that conflicts with the English behavior. 🤖 Prompt for AI AgentsSources: Path instructions, Learnings |
||
| | `alias?` | `string` | — | 可选的公开 model id,用于替代规范化的选择器 slug。 | | ||
|
|
||
| ```json | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| import type { ComboTarget } from "./combo-workspace-data"; | ||
| import type { ModelOption } from "./components/combo-workspace-types"; | ||
|
|
||
| /** Whether every complete target advertises image input. */ | ||
| export function comboImagesSupported(targets: ComboTarget[], models: ModelOption[]): boolean { | ||
| const complete = targets.filter((target) => target.provider.trim() && target.model.trim()); | ||
| if (complete.length === 0) return false; | ||
| return complete.every((target) => { | ||
| const model = models.find( | ||
| (row) => row.provider === target.provider.trim() && row.id === target.model.trim(), | ||
| ); | ||
| return !!model?.inputModalities?.includes("image"); | ||
| }); | ||
| } | ||
|
Comment on lines
+5
to
+14
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win Add a unit test for the fail-closed branches of No test file covers this function in the current batch. This function gates whether the image-input toggle appears enabled in the workspace UI, so a regression here (for example, accidentally using 🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,15 @@ export type ComboEffort = "low" | "medium" | "high" | "xhigh" | "max" | "ultra"; | |
|
|
||
| export const COMBO_EFFORTS: ComboEffort[] = ["low", "medium", "high", "xhigh", "max", "ultra"]; | ||
|
|
||
| /** Intersection of per-member effort ladders; unknown ladders contribute no selectable efforts. */ | ||
| /** | ||
| * Intersection of per-member effort ladders for the default-effort picker. | ||
| * - `undefined` or empty ladder = no advertised rungs — does not constrain | ||
| * (failover combos often mix unknown / no-reasoning members with models that | ||
| * do publish a ladder; a single empty ladder must not zero the picker). | ||
| * - non-empty listed efforts = intersect as usual (filtered to COMBO_EFFORTS). | ||
| * When no complete target advertises a non-empty ladder, returns the full ladder. | ||
| * Runtime still omits injection per-target when a concrete ladder rejects the value. | ||
| */ | ||
| export function intersectComboEfforts( | ||
| targets: readonly ComboTarget[], | ||
| modelEfforts: ReadonlyMap<string, readonly string[] | undefined>, | ||
|
|
@@ -20,22 +28,41 @@ export function intersectComboEfforts( | |
| for (const target of complete) { | ||
| const key = `${target.provider.trim()}/${target.model.trim()}`; | ||
| const listed = modelEfforts.get(key); | ||
| // Missing metadata must not invent a full ladder — runtime omits the combo default when | ||
| // supportedLadderFor is undefined (#488 / Codex review). | ||
| const member: string[] = listed === undefined | ||
| ? [] | ||
| : listed.filter((effort) => effortSet.has(effort)); | ||
| // Skip unknown / empty ladders so they do not empty the picker. | ||
| if (listed === undefined || listed.length === 0) continue; | ||
|
Comment on lines
+31
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a selected model has a known empty ladder (for example a AGENTS.md reference: gui/AGENTS.md:L9-L10 Useful? React with 👍 / 👎. |
||
| const member = listed.filter((effort) => effortSet.has(effort)); | ||
| if (member.length === 0) continue; | ||
| if (common === null) { | ||
| common = member; | ||
| } else { | ||
| const memberSet = new Set(member); | ||
| common = common.filter((effort) => memberSet.has(effort)); | ||
| } | ||
| } | ||
| const commonSet = new Set(common ?? []); | ||
| // No constraining ladders among complete targets → full selectable set. | ||
| if (common === null) return [...COMBO_EFFORTS]; | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| const commonSet = new Set(common); | ||
| return COMBO_EFFORTS.filter((effort) => commonSet.has(effort)); | ||
| } | ||
|
|
||
| /** | ||
| * True when any complete target has no advertised effort ladder (undefined or | ||
| * empty). The picker treats those as wildcards, but runtime still fails closed | ||
| * for unknown ladders — surface that so options are not presented as fully known. | ||
| */ | ||
| export function comboHasUnknownEffortTargets( | ||
| targets: readonly ComboTarget[], | ||
| modelEfforts: ReadonlyMap<string, readonly string[] | undefined>, | ||
| ): boolean { | ||
| const complete = targets.filter((t) => t.provider.trim() && t.model.trim()); | ||
| if (complete.length === 0) return false; | ||
| return complete.some((target) => { | ||
| const key = `${target.provider.trim()}/${target.model.trim()}`; | ||
| const listed = modelEfforts.get(key); | ||
| return listed === undefined || listed.length === 0; | ||
| }); | ||
| } | ||
|
|
||
| export interface ComboTarget { | ||
| provider: string; | ||
| model: string; | ||
|
|
@@ -64,6 +91,11 @@ export interface ComboItem { | |
| strategy: ComboStrategy; | ||
| stickyLimit: number; | ||
| defaultEffort: ComboEffort | null; | ||
| /** | ||
| * Image/multimodal policy. Default `auto` (checked) keeps the target | ||
| * intersection; `disabled` forces text-only. | ||
| */ | ||
| imageInput?: "auto" | "disabled"; | ||
| targets: ComboTarget[]; | ||
| } | ||
|
|
||
|
|
@@ -123,6 +155,10 @@ export function normalizeWeight(raw: unknown): number | undefined { | |
| : undefined; | ||
| } | ||
|
|
||
| export function normalizeImageInput(raw: unknown): "auto" | "disabled" { | ||
| return raw === "disabled" ? "disabled" : "auto"; | ||
| } | ||
|
|
||
| export function parseComboList(payload: unknown): ComboItem[] { | ||
| if (!payload || typeof payload !== "object") return []; | ||
| const rows = (payload as { combos?: unknown }).combos; | ||
|
|
@@ -153,6 +189,7 @@ export function parseComboList(payload: unknown): ComboItem[] { | |
| strategy: normalizeStrategy(r.strategy), | ||
| stickyLimit: normalizeStickyLimit(r.stickyLimit), | ||
| defaultEffort: normalizeDefaultEffort(r.defaultEffort), | ||
| imageInput: normalizeImageInput(r.imageInput), | ||
| targets, | ||
| }); | ||
| } | ||
|
|
@@ -210,6 +247,7 @@ export function draftEquals(a: ComboItem, b: ComboItem): boolean { | |
| || a.strategy !== b.strategy | ||
| || a.stickyLimit !== b.stickyLimit | ||
| || a.defaultEffort !== b.defaultEffort | ||
| || (a.imageInput ?? "auto") !== (b.imageInput ?? "auto") | ||
| ) return false; | ||
| if (a.targets.length !== b.targets.length) return false; | ||
| return a.targets.every((t, i) => { | ||
|
|
@@ -226,6 +264,7 @@ export function toPutBody(item: ComboItem, options: { renameFrom?: string } = {} | |
| strategy: ComboStrategy; | ||
| stickyLimit?: number; | ||
| defaultEffort: ComboEffort | null; | ||
| imageInput?: "disabled"; | ||
| alias?: string; | ||
| }; | ||
| } { | ||
|
|
@@ -239,6 +278,7 @@ export function toPutBody(item: ComboItem, options: { renameFrom?: string } = {} | |
| strategy: item.strategy, | ||
| defaultEffort: item.defaultEffort, | ||
| ...(item.strategy === "round-robin" ? { stickyLimit: item.stickyLimit } : {}), | ||
| ...(item.imageInput === "disabled" ? { imageInput: "disabled" as const } : {}), | ||
| ...(item.alias && item.alias.trim() ? { alias: item.alias.trim() } : {}), | ||
| }, | ||
| }; | ||
|
|
@@ -326,6 +366,7 @@ export function emptyDraft(id = ""): ComboItem { | |
| strategy: "failover", | ||
| stickyLimit: 1, | ||
| defaultEffort: null, | ||
| imageInput: "auto", | ||
| targets: [newComboTarget()], | ||
| }; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add
imageInputto the configuration reference table.This section documents
imageInput, but the field table at Lines 261-268 omits it. Users who rely on the configuration reference will not see the supported values or default. Add the sameimageInputrow that exists indocs-site/src/content/docs/zh-cn/guides/combos.mdLine 217.As per path instructions, user-facing documentation must stay in sync with actual CLI and API behavior.
🤖 Prompt for AI Agents
Source: Path instructions