Skip to content

fix(catalog): custom model rows inherit provider reasoning metadata #965

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.

Sign up for GitHub

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

Closed
Yuxin-Qiao wants to merge 1 commit into from
+82 −4
Closed

fix(catalog): custom model rows inherit provider reasoning metadata #965

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 26 additions & 4 deletions src/codex/catalog/provider-fetch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -782,6 +782,9 @@ async function gatherRoutedModelsUncached(
// Enriched (registry-hydrated) provider clones, keyed by name — the same view used above so
// custom rows get the same noVisionModels / inputModalities treatment as discovered rows.
const enrichedByName = new Map(activeProviders);
// Provider-derived rows keyed by their Codex-facing slug: a custom override replaces the row
// with the same slug below, so that row's provider capability metadata is the inheritance source.
const replacedByRoutedSlug = new Map(all.map(model => [routedSlug(model.provider, model.id), model]));

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Match inherited metadata by exact model identity

When a provider exposes colliding native IDs such as a-b and a/b, both produce the same routed slug, and the preceding sort causes this map to retain the slash row. A custom override for the plain a-b model therefore inherits the other model's reasoning ladder, context, and capabilities, even though resolveSlugAliasCollisions() deliberately catalogs the plain-hyphen model as the winner. Index the provider-derived rows by exact provider/native ID first, and use a routed-slug fallback only when that slug is unambiguous.

Useful? React with 👍 / 👎.

const customModels = (config.customModels ?? []).map(cm => {
const rawProvider = config.providers[cm.provider];
const supportsReasoningSummaries = configuredReasoningSummarySupport(rawProvider, cm.modelId);
Expand All @@ -794,19 +797,38 @@ async function gatherRoutedModelsUncached(
...(cm.inputModalities ? { inputModalities: cm.inputModalities } : {}),
...(typeof supportsReasoningSummaries === "boolean" ? { supportsReasoningSummaries } : {}),
};
// #962: the dedupe below drops the provider-derived row this custom row replaces. Inherit that
// row's provider capability metadata (reasoning ladder, default effort, parallel tool calls,
// context, ...) so the generated catalog keeps advertising what the router actually provides.
// Explicit custom fields win by construction; this only fills gaps. Without it a
// noReasoningModels model loses its empty ladder and the catalog synthesizes the generic one,
// which Codex then rejects for spawn_agent with effort "none".
const replaced = replacedByRoutedSlug.get(routedSlug(cm.provider, cm.modelId));
const merged: CatalogModel = replaced ? {
...base,
...(base.contextWindow === undefined && replaced.contextWindow !== undefined ? { contextWindow: replaced.contextWindow } : {}),
...(base.maxInputTokens === undefined && replaced.maxInputTokens !== undefined ? { maxInputTokens: replaced.maxInputTokens } : {}),
...(base.inputModalities === undefined && replaced.inputModalities !== undefined ? { inputModalities: replaced.inputModalities } : {}),
...(base.reasoningEfforts === undefined && replaced.reasoningEfforts !== undefined ? { reasoningEfforts: replaced.reasoningEfforts } : {}),
...(base.defaultReasoningEffort === undefined && replaced.defaultReasoningEffort !== undefined ? { defaultReasoningEffort: replaced.defaultReasoningEffort } : {}),
...(base.parallelToolCalls === undefined && replaced.parallelToolCalls !== undefined ? { parallelToolCalls: replaced.parallelToolCalls } : {}),
...(base.supportsVerbosity === undefined && replaced.supportsVerbosity !== undefined ? { supportsVerbosity: replaced.supportsVerbosity } : {}),
...(base.supportsReasoningSummaries === undefined && replaced.supportsReasoningSummaries !== undefined ? { supportsReasoningSummaries: replaced.supportsReasoningSummaries } : {}),
...(base.capabilities === undefined && replaced.capabilities !== undefined ? { capabilities: replaced.capabilities } : {}),
} : base;
Comment on lines +806 to +818

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Derive capabilities when no provider row exists

When a custom model is intentionally added outside the provider's discovered/static list—a supported case already exercised by the renamed-model custom-model test—this lookup returns undefined, so the row still ignores noReasoningModels, modelReasoningEfforts, defaults, and adapter capabilities and buildCatalogEntries() synthesizes the generic reasoning ladder. This leaves the reported spawn-agent failure unfixed for the normal use case where customModels supplies an otherwise unlisted model; derive the missing capability fields through the provider hint flow even when there is no replaced row, while retaining explicit custom context/modalities.

AGENTS.md reference: src/AGENTS.md:L18-L18

Useful? React with 👍 / 👎.

// Vision-sidecar coverage ONLY: if the custom model is in the enriched provider's
// noVisionModels, advertise image input so the Codex app lets images reach the sidecar
// (#349/#344). Deliberately NOT the full applyProviderConfigHints pass — custom rows are a
// user override, so their explicit contextWindow / inputModalities / reasoning fields must be
// preserved verbatim (the hint pass would cap context and overwrite modalities from registry).
const enrichedProvider = enrichedByName.get(cm.provider) ?? rawProvider;
if (enrichedProvider && modelInList(enrichedProvider.noVisionModels, base.id)) {
const current = base.inputModalities ?? ["text"];
if (enrichedProvider && modelInList(enrichedProvider.noVisionModels, merged.id)) {
const current = merged.inputModalities ?? ["text"];
if (!current.includes("image")) {
return { ...base, inputModalities: [...current, "image"] };
return { ...merged, inputModalities: [...current, "image"] };
}
}
return base;
return merged;
});
// Custom rows override discovered rows that encode to the same Codex-facing slug.
const customKeys = new Set(customModels.map(c => routedSlug(c.provider, c.id)));
Expand Down
56 changes: 56 additions & 0 deletions tests/codex-catalog.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -848,6 +848,62 @@ describe("configured CatalogModel displayName -> catalog display_name", () => {
});
});

test("a custom row inherits provider reasoning metadata from the provider-derived row it replaces (#962)", async () => {
clearModelCache("ollama");
const originalFetch = globalThis.fetch;
globalThis.fetch = (() => { throw new Error("fetch should not be called"); }) as typeof fetch;
try {
const models = await gatherRoutedModels({
port: 10100,
defaultProvider: "ollama",
providers: {
ollama: {
baseUrl: "http://localhost:11434/v1",
adapter: "openai-chat",
authMode: "key",
liveModels: false,
models: ["qwen-coder-3b"],
selectedModels: ["qwen-coder-3b"],
noReasoningModels: ["qwen-coder-3b"],
modelReasoningEfforts: { "qwen-coder-3b": [] },
},
},
customModels: [
{
id: "cm-962",
provider: "ollama",
modelId: "qwen-coder-3b",
displayName: "Qwen Coder 3B (local)",
contextWindow: 32768,
inputModalities: ["text"],
addedAt: "2026-01-01T00:00:00.000Z",
},
],
});

// Explicit custom fields stay verbatim; provider capability metadata is inherited from the
// replaced provider-derived row (noReasoningModels -> empty reasoning ladder, openai-chat
// adapter -> parallel tool calls).
const custom = models.find(m => m.provider === "ollama" && m.id === "qwen-coder-3b");
expect(custom?.displayName).toBe("Qwen Coder 3B (local)");
expect(custom?.contextWindow).toBe(32768);
expect(custom?.inputModalities).toEqual(["text"]);
expect(custom?.reasoningEfforts).toEqual([]);
expect(custom?.parallelToolCalls).toBe(true);

const entries = buildCatalogEntries(nativeTemplate(), [], models);
const row = entries.find(e => e.slug === "ollama/qwen-coder-3b");
expect(row?.display_name).toBe("Qwen Coder 3B (local)");
// The catalog must expose no reasoning levels and no default reasoning level for this model;
// the generic low..ultra ladder and the medium default must not be synthesized.
expect(row?.supported_reasoning_levels).toEqual([]);
expect(row?.default_reasoning_level).toBeUndefined();
Comment on lines +851 to +900

@coderabbitai coderabbitai Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for an inherited non-empty default reasoning effort.

This fixture gives the provider-derived row an empty reasoningEfforts list and no defaultReasoningEffort. Therefore, the test cannot detect a regression in src/codex/catalog/provider-fetch.ts line 813.

Add a second provider/custom pair with configured reasoning efforts and modelDefaultReasoningEfforts. Assert both CatalogModel.defaultReasoningEffort and the generated default_reasoning_level. Keep the current no-reasoning case for issue #962.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/codex-catalog.test.ts` around lines 851 - 900, Add a second
provider/custom model fixture in the existing test to cover inherited non-empty
reasoning metadata, configuring provider reasoning efforts and
modelDefaultReasoningEfforts. Assert the replaced custom CatalogModel preserves
defaultReasoningEffort and that buildCatalogEntries emits the corresponding
default_reasoning_level, while retaining the current empty-reasoning assertions
for issue `#962`.

} finally {
globalThis.fetch = originalFetch;
clearModelCache("ollama");
}
});

function openAiApiCatalogConfig(overrides: Record<string, unknown> = {}): OcxConfig {
return {
port: 10100,
Expand Down
Loading