From 68d44b4664e4c58ff756e08bd1a2dd419c41212c Mon Sep 17 00:00:00 2001 From: TooSpace Date: Fri, 14 Aug 2026 13:43:47 +0800 Subject: [PATCH] feat(catalog): add modelPickerOrder to customize the Codex model-picker order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Large routed catalogs (10-20+ models across several providers) have no supported way to control the Codex model-picker display order beyond the 5-slot subagentModels list: every non-featured routed row is emitted at the same flat priority, so the picker order is undefined and reshuffles on each catalog rebuild (ocx sync / service restart / upgrade). Add an optional, display-only config.modelPickerOrder: string[]. Listed routed / slugs are shown in array order in the picker; unlisted rows and subagentModels-featured rows keep their positions. When unset, catalog priority is byte-identical to before (the codex-catalog golden oracle is unchanged). Display and spawn_agent candidacy are fully decoupled: modelPickerOrder rewrites only the Codex-visible `priority`, while each moved row records its natural priority in an OpenCodex-private catalog field (opencodex_spawn_priority) that effectiveSubagentRoster uses to pick candidates. The spawn_agent candidate set is therefore provably unchanged by any display reordering — even reversing every row. Codex ignores the unknown field (same as opencodex_catalog_kind), so this is purely a user-facing picker feature. Fixes #1649 --- src/codex/catalog/sync.ts | 76 ++++++- src/codex/convergence.ts | 2 + src/types.ts | 14 ++ .../codex-catalog-model-picker-order.test.ts | 188 ++++++++++++++++++ 4 files changed, 276 insertions(+), 4 deletions(-) create mode 100644 tests/codex-catalog-model-picker-order.test.ts diff --git a/src/codex/catalog/sync.ts b/src/codex/catalog/sync.ts index e3a9e5689..1ca390924 100644 --- a/src/codex/catalog/sync.ts +++ b/src/codex/catalog/sync.ts @@ -67,6 +67,18 @@ import { accountBoundNativeDisplayName, CODEX_ACCOUNT_BOUND_CATALOG_KIND, truste export const MAX_SPAWN_AGENT_MODEL_OVERRIDES = 5; +// Base for config.modelPickerOrder display priorities (#1649). modelPickerOrder is a DISPLAY-ONLY +// reordering of the Codex model picker: it rewrites a row's Codex-visible `priority` but never the +// spawn_agent candidate window. The window is derived from SPAWN_PRIORITY_FIELD (the natural +// priority captured before the override), so display order and spawn candidates are decoupled. +export const PICKER_ORDER_PRIORITY_BASE = 1_000; + +// OpenCodex-private catalog field: the spawn_agent candidate priority a row would have WITHOUT +// modelPickerOrder. Codex ignores unknown catalog fields (same as opencodex_catalog_kind), so this +// is invisible to Codex; effectiveSubagentRoster reads it so a display reorder cannot change which +// rows are spawn_agent candidates. Absent on rows modelPickerOrder did not move. +export const SPAWN_PRIORITY_FIELD = "opencodex_spawn_priority"; + export type SpawnAgentSurface = "v1" | "v2"; export type SubagentRosterExclusionReason = @@ -144,10 +156,17 @@ export function effectiveSubagentRoster( .filter(({ entry }) => entry.visibility === "list") .filter(({ entry }) => surface !== "v2" || isEligibleV2SubagentEntry(entry)) .sort((left, right) => { - const leftPriority = typeof left.entry.priority === "number" && Number.isFinite(left.entry.priority) - ? left.entry.priority : Number.MAX_SAFE_INTEGER; - const rightPriority = typeof right.entry.priority === "number" && Number.isFinite(right.entry.priority) - ? right.entry.priority : Number.MAX_SAFE_INTEGER; + // Spawn candidates rank by the natural priority (SPAWN_PRIORITY_FIELD when present), so a + // modelPickerOrder display reorder (#1649) can never change candidate membership. Rows the + // override did not move fall back to their Codex-visible `priority`. + const spawnPriorityOf = (entry: RawEntry): number => { + const spawn = entry[SPAWN_PRIORITY_FIELD]; + if (typeof spawn === "number" && Number.isFinite(spawn)) return spawn; + return typeof entry.priority === "number" && Number.isFinite(entry.priority) + ? entry.priority : Number.MAX_SAFE_INTEGER; + }; + const leftPriority = spawnPriorityOf(left.entry); + const rightPriority = spawnPriorityOf(right.entry); return leftPriority - rightPriority || left.index - right.index; }) .slice(0, MAX_SPAWN_AGENT_MODEL_OVERRIDES); @@ -362,6 +381,8 @@ export interface ObservedCatalogEntryBuildInput { readonly gptSlugs: readonly string[]; readonly goModels: readonly CatalogModel[]; readonly featured?: readonly string[]; + /** Optional full picker ordering (config.modelPickerOrder); orders non-featured rows. */ + readonly modelPickerOrder?: readonly string[]; readonly wsEnabled: boolean; readonly multiAgentMode: MultiAgentMode; readonly exactComboSlugs: ReadonlySet; @@ -416,6 +437,7 @@ export function buildCatalogEntriesFromObservedState({ gptSlugs, goModels, featured, + modelPickerOrder, wsEnabled, multiAgentMode, exactComboSlugs, @@ -433,6 +455,37 @@ export function buildCatalogEntriesFromObservedState({ // it sorts to the front. This works for native gpt slugs AND routed slugs alike. const rank = new Map((featured ?? []).map((slug, i) => [slug, i] as const)); const priorityStride = Math.max(accountSelectors.length, 1); + // Optional full picker order (#1649). Independent of the 5-slot spawn_agent cap: it only + // rewrites the Codex-visible display `priority` of listed non-featured routed rows so a >5 + // catalog stays put across rebuilds. Featured rows keep their existing 0..N-1 band; when + // modelPickerOrder is unset the helper is a no-op and every priority below is byte-identical to + // before. The spawn_agent candidate window is derived separately from SPAWN_PRIORITY_FIELD, so + // this display reorder cannot change which rows are spawn candidates. + const pickerOrder = (modelPickerOrder ?? []).filter(id => typeof id === "string" && id.length > 0); + const pickerOrderRank = new Map(pickerOrder.map((slug, i) => [slug, i] as const)); + const pickerOrderActive = pickerOrder.length > 0; + // The display band reuses the existing high priority tier (>= PICKER_ORDER_PRIORITY_BASE, the + // same 1_000+ neighborhood account rows occupy), keeping listed rows visually after the featured + // band. Candidate membership does not depend on this — see SPAWN_PRIORITY_FIELD. + /** + * Priority for a non-featured routed row that is explicitly LISTED in modelPickerOrder. Listed + * slugs sort in declared order within the high picker-order display tier + * (>= PICKER_ORDER_PRIORITY_BASE). This sets the Codex-visible `priority` only; the caller records + * the row's natural priority in SPAWN_PRIORITY_FIELD so the spawn_agent candidate window is + * unchanged. Returns undefined when the feature is off or the row is not listed, so those rows + * keep their original assignment (default 5 / account 1_000+) untouched. + * + * Scope: only the generic routed `/` rows call this (see the goModels loop + * below). Native passthrough rows and account-qualified native rows keep their own priority + * logic and are intentionally not reordered here — this matches the documented contract on + * OcxConfig.modelPickerOrder (route native ordering through subagentModels instead). + */ + const pickerOrderPriority = (slug: string, altSlug?: string): number | undefined => { + if (!pickerOrderActive) return undefined; + const hit = pickerOrderRank.get(slug) ?? (altSlug !== undefined ? pickerOrderRank.get(altSlug) : undefined); + if (hit === undefined) return undefined; + return PICKER_ORDER_PRIORITY_BASE + hit * priorityStride; + }; const out: RawEntry[] = []; const nativeEntries: RawEntry[] = []; const collisionSkipped = resolveSlugAliasCollisions([...goModels]); @@ -537,11 +590,24 @@ export function buildCatalogEntriesFromObservedState({ } // Featured picks may be stored raw (legacy) or encoded — honor both. const rankHit = rank.get(slug) ?? rank.get(`${m.provider}/${m.id}`); + // Natural priority: what the row would get WITHOUT modelPickerOrder. This is the value the + // spawn_agent candidate window is derived from (see effectiveSubagentRoster), so it must never + // move when modelPickerOrder reorders the picker. if (rankHit !== undefined) e.priority = rankHit * priorityStride; else if (accountSelectors.length > 0) { // Keep the generated account rows together in Codex's priority-sorted flat picker. e.priority = 1_000 + (typeof e.priority === "number" ? e.priority : 5); } + // #1649: modelPickerOrder is a DISPLAY-ONLY override. Record the natural priority spawn_agent + // must keep using, then let modelPickerOrder move only the Codex-visible `priority`. Featured + // rows are never overridden (their rank is authoritative for both display and spawn). + if (rankHit === undefined) { + const pickerPriority = pickerOrderPriority(slug, `${m.provider}/${m.id}`); + if (pickerPriority !== undefined) { + e[SPAWN_PRIORITY_FIELD] = typeof e.priority === "number" ? e.priority : 5; + e.priority = pickerPriority; + } + } out.push(e); } // Central capability override (phase 120.4): the advertised flag must match the implemented WS @@ -1324,6 +1390,7 @@ function writeRetainedCatalogSync({ const enabledGo = filterCatalogVisibleModels(goModels, config); const featured = config.subagentModels ?? []; const orderedGoModels = orderForSubagents(enabledGo, featured); // stable tie-break among equal priorities + const modelPickerOrder = config.modelPickerOrder ?? []; const multiAgentMode: MultiAgentMode = config.multiAgentMode === "v1" || config.multiAgentMode === "v2" ? config.multiAgentMode : "default"; const exactComboSlugs = exactComboCatalogSlugs(config); const suppressedBareNativeSlugs = desktopAllowlistSuppressedNativeSlugs(config); @@ -1355,6 +1422,7 @@ function writeRetainedCatalogSync({ gptSlugs: [], goModels: orderedGoModels, featured, + modelPickerOrder, wsEnabled, multiAgentMode, exactComboSlugs, diff --git a/src/codex/convergence.ts b/src/codex/convergence.ts index 4054e5000..de35535f0 100644 --- a/src/codex/convergence.ts +++ b/src/codex/convergence.ts @@ -225,6 +225,7 @@ function prepareCatalog( const enabled = filterCatalogVisibleModels(routedModels, config); const featured = config.subagentModels ?? []; const ordered = orderForSubagents(enabled, featured); + const modelPickerOrder = config.modelPickerOrder ?? []; const multiAgentMode = config.multiAgentMode === "v1" || config.multiAgentMode === "v2" ? config.multiAgentMode : "default"; const exactComboSlugs = exactComboCatalogSlugs(config); @@ -255,6 +256,7 @@ function prepareCatalog( gptSlugs: [], goModels: ordered, featured, + modelPickerOrder, wsEnabled: websocketsEnabled(config), multiAgentMode, exactComboSlugs, diff --git a/src/types.ts b/src/types.ts index d24811f43..69aae6eb3 100644 --- a/src/types.ts +++ b/src/types.ts @@ -637,6 +637,20 @@ export interface OcxConfig { * into a selector-qualified group; Codex still advertises only the first 5 visible rows. */ subagentModels?: string[]; + /** + * Optional full picker ordering for the Codex model catalog, independent of the + * 5-slot `subagentModels` spawn_agent cap. DISPLAY-ONLY: it controls the visual order of + * the Codex model picker for large routed catalogs (10-20+ models) that would otherwise sort + * arbitrarily and reshuffle on every rebuild. Values are routed `/` catalog + * slugs (matched by exact slug or `provider/id`); native OpenAI passthrough rows and + * account-qualified native rows are not reordered (order native rows via `subagentModels`). + * Listed routed rows appear in array order; rows not listed keep their normal display order. + * `subagentModels`-featured rows keep their top position. When unset or empty, catalog + * priority is unchanged. This changes ONLY what the user sees in the picker: the spawn_agent + * candidate set is derived from each row's natural priority and is provably unaffected, even + * when every routed row is listed (see opencodex_spawn_priority / effectiveSubagentRoster). + */ + modelPickerOrder?: string[]; /** * Priority-ordered fallback models for spawned sub-agents. When the requested * model is quota-exhausted or recently failed, opencodex rewrites the child diff --git a/tests/codex-catalog-model-picker-order.test.ts b/tests/codex-catalog-model-picker-order.test.ts new file mode 100644 index 000000000..1bbc02a83 --- /dev/null +++ b/tests/codex-catalog-model-picker-order.test.ts @@ -0,0 +1,188 @@ +import { describe, expect, test } from "bun:test"; +import { + buildCatalogEntriesFromObservedState, + effectiveSubagentRoster, + MAX_SPAWN_AGENT_MODEL_OVERRIDES, +} from "../src/codex/catalog/sync"; +import type { CatalogModel } from "../src/types"; + +// #1649: config.modelPickerOrder assigns a deterministic priority band to non-featured routed +// rows so a catalog with more than 5 routed models keeps a stable picker order across rebuilds, +// independent of the 5-slot subagentModels spawn_agent cap. + +function template(): Record { + return { + slug: "gpt-5.5", + display_name: "gpt-5.5", + description: "Native GPT model", + priority: 1, + visibility: "list", + tool_mode: "code", + }; +} + +const goModels = [ + { id: "glm-5.2", provider: "jd-chat", owned_by: "jd" }, + { id: "kimi-k3", provider: "jd-chat", owned_by: "jd" }, + { id: "deepseek-v4-pro", provider: "tyler", owned_by: "tyler" }, + { id: "sonnet-5", provider: "jd-claude", owned_by: "jd" }, +] as unknown as CatalogModel[]; + +function build(overrides: { featured?: string[]; modelPickerOrder?: string[] }) { + const entries = buildCatalogEntriesFromObservedState({ + template: template() as never, + gptSlugs: [], + goModels, + featured: overrides.featured, + modelPickerOrder: overrides.modelPickerOrder, + wsEnabled: false, + multiAgentMode: "default", + exactComboSlugs: new Set(), + accountSelectors: [], + suppressedBareNativeSlugs: new Set(), + disabledNativeAccountSlugs: new Set(), + multiAgentV2Enabled: false, + }); + return Object.fromEntries(entries.map(e => { + const r = e as Record; + return [r.slug as string, r.priority as number]; + })) as Record; +} + +describe("modelPickerOrder (#1649)", () => { + test("unset leaves every non-featured routed row at the flat default priority", () => { + const p = build({}); + expect(p["jd-chat/glm-5.2"]).toBe(5); + expect(p["jd-chat/kimi-k3"]).toBe(5); + expect(p["tyler/deepseek-v4-pro"]).toBe(5); + expect(p["jd-claude/sonnet-5"]).toBe(5); + }); + + test("listed rows sort among themselves in declared order, in the high picker tier", () => { + const p = build({ + modelPickerOrder: [ + "tyler/deepseek-v4-pro", + "jd-chat/kimi-k3", + "jd-chat/glm-5.2", + ], + }); + // Declared order is honored among the listed rows. + expect(p["tyler/deepseek-v4-pro"]).toBeLessThan(p["jd-chat/kimi-k3"]); + expect(p["jd-chat/kimi-k3"]).toBeLessThan(p["jd-chat/glm-5.2"]); + // Listed rows occupy the high picker tier (>= 1000); an unlisted, non-featured row keeps its + // default priority (5) and therefore is NOT reordered by modelPickerOrder. + expect(p["tyler/deepseek-v4-pro"]).toBeGreaterThanOrEqual(1000); + expect(p["jd-claude/sonnet-5"]).toBe(5); + }); + + test("featured rows keep their top priority ahead of the picker-order band", () => { + const p = build({ + featured: ["jd-claude/sonnet-5"], + modelPickerOrder: ["tyler/deepseek-v4-pro", "jd-chat/kimi-k3"], + }); + // Featured wins outright (priority 0). + expect(p["jd-claude/sonnet-5"]).toBe(0); + // Picker-order rows come after the featured band. + expect(p["tyler/deepseek-v4-pro"]).toBeGreaterThan(p["jd-claude/sonnet-5"]); + expect(p["tyler/deepseek-v4-pro"]).toBeLessThan(p["jd-chat/kimi-k3"]); + }); + + // Regression for the review on #1666: modelPickerOrder must not change spawn_agent candidate + // eligibility. spawn_agent takes the first MAX_SPAWN_AGENT_MODEL_OVERRIDES picker rows by + // ascending priority. The picker-order band lives in the high (>= 1_000) tier, so featured + // rows (0..N-1) and any default-tier routed rows (priority 5) fill the candidate window first; + // a row that is ONLY placed by modelPickerOrder does not displace a default-tier candidate. + test("picker-order-only rows do not displace default-tier spawn_agent candidates", () => { + const manyRouted = [ + // Not in modelPickerOrder -> stay at default priority 5 -> fill the candidate window. + { id: "unlisted-a", provider: "jd-chat", owned_by: "jd" }, + { id: "unlisted-b", provider: "jd-chat", owned_by: "jd" }, + { id: "unlisted-c", provider: "jd-chat", owned_by: "jd" }, + { id: "unlisted-d", provider: "jd-chat", owned_by: "jd" }, + { id: "unlisted-e", provider: "jd-chat", owned_by: "jd" }, + // Placed only by modelPickerOrder -> high tier -> must stay out of the candidate window. + { id: "deepseek-v4-pro", provider: "tyler", owned_by: "tyler" }, + { id: "kimi-k3", provider: "jd-chat", owned_by: "jd" }, + ] as unknown as CatalogModel[]; + const order = ["tyler/deepseek-v4-pro", "jd-chat/kimi-k3"]; + const entries = buildCatalogEntriesFromObservedState({ + template: template() as never, + gptSlugs: [], + goModels: manyRouted, + featured: [], + modelPickerOrder: order, + wsEnabled: false, + multiAgentMode: "default", + exactComboSlugs: new Set(), + accountSelectors: [], + suppressedBareNativeSlugs: new Set(), + disabledNativeAccountSlugs: new Set(), + multiAgentV2Enabled: false, + }); + const candidateSlugs = effectiveSubagentRoster([], "default", entries).candidates.map(c => c.model); + expect(candidateSlugs.length).toBe(MAX_SPAWN_AGENT_MODEL_OVERRIDES); + // The picker-order-only rows are pushed to the high tier and never enter the window. + expect(candidateSlugs).not.toContain("tyler/deepseek-v4-pro"); + expect(candidateSlugs).not.toContain("jd-chat/kimi-k3"); + }); + + // Documents the scope boundary raised in review: modelPickerOrder targets routed + // / rows only. A bare native slug listed here must NOT reorder its native + // passthrough row (native ordering goes through subagentModels). + test("a bare native slug in modelPickerOrder does not reorder its native row", () => { + const entries = buildCatalogEntriesFromObservedState({ + template: template() as never, + gptSlugs: ["gpt-5.5", "gpt-5.4"], + goModels: [{ id: "glm-5.2", provider: "jd-chat", owned_by: "jd" }] as unknown as CatalogModel[], + featured: [], + modelPickerOrder: ["gpt-5.4", "jd-chat/glm-5.2"], + wsEnabled: false, + multiAgentMode: "default", + exactComboSlugs: new Set(), + accountSelectors: [], + suppressedBareNativeSlugs: new Set(), + disabledNativeAccountSlugs: new Set(), + multiAgentV2Enabled: false, + }); + const p = Object.fromEntries((entries as Record[]).map(e => [e.slug as string, e.priority as number])); + // The native row keeps its native priority (9), untouched by modelPickerOrder. + expect(p["gpt-5.4"]).toBe(9); + // The routed row IS placed in the high picker tier. + expect(p["jd-chat/glm-5.2"]).toBeGreaterThanOrEqual(1000); + }); + + // Decisive regression for #1666: even when EVERY routed row is listed in modelPickerOrder in + // reverse order (exhausting the default tier entirely), the spawn_agent candidate SET is + // unchanged. This is the case a single display-priority band cannot satisfy; the candidate + // window is derived from the natural priority (opencodex_spawn_priority), not display order. + test("candidate set is unchanged when all routed rows are listed in reverse order", () => { + const sixRouted = [ + { id: "m1", provider: "jd-chat", owned_by: "jd" }, + { id: "m2", provider: "jd-chat", owned_by: "jd" }, + { id: "m3", provider: "jd-chat", owned_by: "jd" }, + { id: "m4", provider: "jd-chat", owned_by: "jd" }, + { id: "m5", provider: "jd-chat", owned_by: "jd" }, + { id: "m6", provider: "jd-chat", owned_by: "jd" }, + ] as unknown as CatalogModel[]; + const buildWith = (modelPickerOrder?: string[]) => buildCatalogEntriesFromObservedState({ + template: template() as never, + gptSlugs: [], + goModels: sixRouted, + featured: [], + modelPickerOrder, + wsEnabled: false, + multiAgentMode: "default", + exactComboSlugs: new Set(), + accountSelectors: [], + suppressedBareNativeSlugs: new Set(), + disabledNativeAccountSlugs: new Set(), + multiAgentV2Enabled: false, + }); + const baseline = effectiveSubagentRoster([], "default", buildWith(undefined)).candidates.map(c => c.model); + const reversed = ["jd-chat/m6", "jd-chat/m5", "jd-chat/m4", "jd-chat/m3", "jd-chat/m2", "jd-chat/m1"]; + const withOrder = effectiveSubagentRoster([], "default", buildWith(reversed)).candidates.map(c => c.model); + // The candidate SET (membership) is identical regardless of display reordering. + expect([...withOrder].sort()).toEqual([...baseline].sort()); + expect(withOrder.length).toBe(MAX_SPAWN_AGENT_MODEL_OVERRIDES); + }); +});