Support per-model Copilot SDK wireApi via provider/model registry metadata #42497
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -343,6 +343,52 @@ function isOpenAIModelName(model) { | |
| return model.startsWith("gpt-") || /^o[134][-.]/.test(model) || model === "o1" || model === "o3" || model === "o4"; | ||
| } | ||
|
|
||
| /** | ||
| * Look up a model entry in the models.json catalog, case-insensitively. | ||
| * | ||
| * @param {object | null | undefined} modelsJson | ||
| * @param {string} modelName | ||
| * @param {string | null | undefined} [providerName] | ||
| * @returns {object | null} | ||
| */ | ||
| function getCatalogModelEntry(modelsJson, modelName, providerName) { | ||
| const model = String(modelName || "") | ||
| .toLowerCase() | ||
| .trim(); | ||
| const provider = String(providerName || "") | ||
| .toLowerCase() | ||
| .trim(); | ||
| if (!model || modelsJson == null || typeof modelsJson !== "object" || Array.isArray(modelsJson)) { | ||
| return null; | ||
| } | ||
| const providers = modelsJson.providers; | ||
| if (!providers || typeof providers !== "object" || Array.isArray(providers)) { | ||
| return null; | ||
| } | ||
| const providerEntries = provider | ||
| ? Object.entries(providers).filter( | ||
| ([name]) => | ||
| String(name || "") | ||
| .toLowerCase() | ||
| .trim() === provider | ||
| ) | ||
| : Object.entries(providers); | ||
| for (const [, providerData] of providerEntries) { | ||
| const models = providerData && typeof providerData === "object" ? providerData.models : null; | ||
| if (!models || typeof models !== "object" || Array.isArray(models)) continue; | ||
| for (const [catalogModel, catalogEntry] of Object.entries(models)) { | ||
| if ( | ||
| String(catalogModel || "") | ||
| .toLowerCase() | ||
| .trim() === model | ||
| ) { | ||
| return catalogEntry && typeof catalogEntry === "object" && !Array.isArray(catalogEntry) ? catalogEntry : null; | ||
| } | ||
| } | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| /** | ||
| * Infer the Copilot SDK provider type for a given endpoint provider name and model name. | ||
| * | ||
|
|
@@ -360,10 +406,10 @@ function isOpenAIModelName(model) { | |
| * | ||
| * @param {string} endpointProvider - The `provider` field from the AWF reflect endpoint entry. | ||
| * @param {string} modelName - The resolved model name to use for heuristic fallback. | ||
| * @param {object | null | undefined} catalogEntryOrModelsJson - Matching models.json catalog entry or full catalog (optional). | ||
| * @returns {"openai" | "azure" | "anthropic"} | ||
| */ | ||
| function inferProviderTypeForModel(endpointProvider, modelName, catalogEntryOrModelsJson) { | ||
| // 1. Endpoint provider name mapping. | ||
| const ep = String(endpointProvider || "") | ||
| .toLowerCase() | ||
|
|
@@ -376,27 +422,15 @@ function inferProviderTypeForModel(endpointProvider, modelName, modelsJson) { | |
| const model = String(modelName || "") | ||
| .toLowerCase() | ||
| .trim(); | ||
| const catalogEntry = | ||
| catalogEntryOrModelsJson && typeof catalogEntryOrModelsJson === "object" && !Array.isArray(catalogEntryOrModelsJson) && "providers" in catalogEntryOrModelsJson | ||
| ? getCatalogModelEntry(catalogEntryOrModelsJson, model) | ||
| : catalogEntryOrModelsJson; | ||
|
|
||
| // 2. Model catalog lookup. | ||
| if (model) { | ||
| const pt = catalogEntry && typeof catalogEntry.provider_type === "string" ? catalogEntry.provider_type.trim() : ""; | ||
| if (pt === "anthropic" || pt === "azure" || pt === "openai") return /** @type {"openai" | "azure" | "anthropic"} */ pt; | ||
| } | ||
|
|
||
| // 3. Well-known model name heuristics. | ||
|
|
@@ -409,6 +443,41 @@ function inferProviderTypeForModel(endpointProvider, modelName, modelsJson) { | |
| return "openai"; | ||
| } | ||
|
|
||
| /** | ||
| * Infer the SDK wire API for a model. | ||
| * | ||
| * Resolution order: | ||
| * 1. For Anthropic provider types: undefined (wireApi ignored by SDK). | ||
| * 2. `models.json` explicit `wire_api`/`wireApi`. | ||
| * 3. Heuristic default for OpenAI/Azure-compatible models: "completions". | ||
| * | ||
| * @param {"openai" | "azure" | "anthropic"} providerType | ||
| * @param {string} modelName | ||
| * @param {object | null | undefined} catalogEntryOrModelsJson | ||
| * @returns {"completions" | "responses" | undefined} | ||
| */ | ||
| function inferWireApiForModel(providerType, modelName, catalogEntryOrModelsJson) { | ||
| if (providerType === "anthropic") { | ||
| return undefined; | ||
| } | ||
| const model = String(modelName || "").trim(); | ||
| if (!model) return undefined; | ||
| const catalogEntry = | ||
| catalogEntryOrModelsJson && typeof catalogEntryOrModelsJson === "object" && !Array.isArray(catalogEntryOrModelsJson) && "providers" in catalogEntryOrModelsJson | ||
| ? getCatalogModelEntry(catalogEntryOrModelsJson, model) | ||
| : catalogEntryOrModelsJson; | ||
| // Keep the camelCase fallback for defensive compatibility with injected catalog | ||
| // objects that bypass the normalized models.json pipeline. | ||
| const rawWireApi = typeof catalogEntry?.wire_api === "string" ? catalogEntry.wire_api : typeof catalogEntry?.wireApi === "string" ? catalogEntry.wireApi : ""; | ||
| const normalizedWireApi = String(rawWireApi || "") | ||
| .toLowerCase() | ||
| .trim(); | ||
| if (normalizedWireApi === "responses" || normalizedWireApi === "completions") { | ||
|
Contributor
There was a problem hiding this comment. [/tdd] The resolution order between catalog The PR description only documents the case where Anthropic is "omitted", but the code's branch ordering allows a catalog entry to override that. 💡 Suggested test to pin the decisionit("catalog wire_api wins over anthropic undefined fallback", () => {
const reflectData = {
endpoints: [{ provider: "anthropic", port: 10001, configured: true, models: ["claude-future"] }],
};
const modelsJson = {
providers: {
"github-copilot": { models: { "claude-future": { provider_type: "anthropic", wire_api: "responses", cost: {} } } },
},
};
expect(resolveCopilotSDKCustomProviderFromReflect({ reflectData, model: "claude-future", modelsJson })).toEqual({
model: "claude-future",
// Assert whether wireApi: "responses" should be present or absent for Anthropic
provider: { type: "anthropic", baseUrl: "(apiproxy/redacted) },
});
});If the catalog should never override the Anthropic fallback, swap the branch order or add an early guard. @copilot please address this. |
||
| return /** @type {"responses" | "completions"} */ normalizedWireApi; | ||
|
Contributor
There was a problem hiding this comment. Anthropic guard is bypassed when catalog has an explicit 💡 Suggested fixMove the Anthropic guard before the catalog lookup so it is unconditional: function inferWireApiForModel(providerType, modelName, modelsJson) {
// Anthropic SDK ignores wireApi; never emit it for Anthropic providers.
if (providerType === "anthropic") return undefined;
const model = String(modelName || "").trim();
if (!model) return undefined;
const catalogEntry = getCatalogModelEntry(modelsJson, model);
const rawWireApi =
catalogEntry && typeof catalogEntry.wire_api === "string"
? catalogEntry.wire_api
: catalogEntry && typeof catalogEntry.wireApi === "string"
? catalogEntry.wireApi
: "";
const normalizedWireApi = String(rawWireApi || "").toLowerCase().trim();
if (normalizedWireApi === "responses" || normalizedWireApi === "completions") {
return normalizedWireApi;
}
return "completions";
}This ensures no catalog misconfiguration can override the Anthropic constraint. |
||
| } | ||
| return "completions"; | ||
| } | ||
|
|
||
| /** | ||
| * Resolve Copilot SDK BYOK custom provider configuration from AWF /reflect data. | ||
| * Chooses a configured endpoint and maps it to a provider base URL and type. | ||
|
|
@@ -424,7 +493,7 @@ function inferProviderTypeForModel(endpointProvider, modelName, modelsJson) { | |
| * modelsJson?: object | null, | ||
| * logger?: (msg: string) => void, | ||
| * }} [options] | ||
| * @returns {{ model: string, provider: { type: "openai" | "azure" | "anthropic", baseUrl: string, wireApi?: "completions" | "responses" } } | null} | ||
| */ | ||
| function resolveCopilotSDKCustomProviderFromReflect(options) { | ||
| const configuredModel = typeof options?.model === "string" ? options.model.trim() : ""; | ||
|
|
@@ -475,11 +544,20 @@ function resolveCopilotSDKCustomProviderFromReflect(options) { | |
| return null; | ||
| } | ||
|
|
||
| const endpointProvider = String(endpoint.provider || ""); | ||
| const catalogProviderName = | ||
| String(endpointProvider || "") | ||
| .toLowerCase() | ||
| .trim() === "copilot" | ||
| ? "github-copilot" | ||
| : endpointProvider; | ||
| const catalogEntry = getCatalogModelEntry(options?.modelsJson ?? null, model, catalogProviderName); | ||
| const providerType = inferProviderTypeForModel(endpointProvider, model, catalogEntry); | ||
| const wireApi = inferWireApiForModel(providerType, model, catalogEntry); | ||
| logger(`sdk-mode: custom provider resolved from awf-reflect (provider=${String(endpoint.provider || "unknown")} type=${providerType} baseUrl=${baseUrl} model=${model}${wireApi ? ` wireApi=${wireApi}` : ""})`); | ||
| return { | ||
| model, | ||
| provider: { type: providerType, baseUrl, ...(wireApi ? { wireApi } : {}) }, | ||
| }; | ||
| } | ||
|
|
||
|
|
@@ -497,7 +575,9 @@ if (typeof module !== "undefined" && module.exports) { | |
| extractModelIds, | ||
| fetchAWFReflect, | ||
| fetchModelsFromUrl, | ||
| getCatalogModelEntry, | ||
| inferProviderTypeForModel, | ||
| inferWireApiForModel, | ||
| resolveCopilotSDKCustomProviderFromReflect, | ||
| }; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,9 @@ const { | |
| extractModelIds, | ||
| fetchAWFReflect, | ||
| fetchModelsFromUrl, | ||
| getCatalogModelEntry, | ||
| inferProviderTypeForModel, | ||
| inferWireApiForModel, | ||
| resolveCopilotSDKCustomProviderFromReflect, | ||
| } = require("./awf_reflect.cjs"); | ||
|
|
||
|
|
@@ -396,14 +398,80 @@ describe("awf_reflect.cjs", () => { | |
| }); | ||
| }); | ||
|
|
||
| describe("getCatalogModelEntry", () => { | ||
| it("matches model names case-insensitively", () => { | ||
| const entry = getCatalogModelEntry( | ||
| { | ||
| providers: { | ||
| "github-copilot": { models: { "gpt-5.5": { provider_type: "openai", wire_api: "responses", cost: {} } } }, | ||
| }, | ||
| }, | ||
| "GPT-5.5" | ||
| ); | ||
| expect(entry).toEqual({ provider_type: "openai", wire_api: "responses", cost: {} }); | ||
| }); | ||
|
|
||
| it("uses the requested provider when duplicate model names exist", () => { | ||
| const modelsJson = { | ||
| providers: { | ||
| openai: { models: { "gpt-5.5": { provider_type: "openai", cost: {} } } }, | ||
| "github-copilot": { models: { "gpt-5.5": { provider_type: "openai", wire_api: "responses", cost: {} } } }, | ||
| }, | ||
| }; | ||
| expect(getCatalogModelEntry(modelsJson, "gpt-5.5", "github-copilot")).toEqual({ | ||
| provider_type: "openai", | ||
| wire_api: "responses", | ||
| cost: {}, | ||
| }); | ||
| expect(getCatalogModelEntry(modelsJson, "gpt-5.5", "openai")).toEqual({ | ||
| provider_type: "openai", | ||
| cost: {}, | ||
| }); | ||
| }); | ||
|
|
||
| it("returns null for invalid catalog entries", () => { | ||
| expect( | ||
| getCatalogModelEntry( | ||
| { | ||
| providers: { | ||
| "github-copilot": { models: { broken: null, arrayish: [] } }, | ||
| }, | ||
| }, | ||
| "broken" | ||
| ) | ||
| ).toBeNull(); | ||
| expect( | ||
| getCatalogModelEntry( | ||
| { | ||
| providers: { | ||
| "github-copilot": { models: { broken: null, arrayish: [] } }, | ||
| }, | ||
| }, | ||
| "arrayish" | ||
| ) | ||
| ).toBeNull(); | ||
| }); | ||
| }); | ||
|
|
||
| describe("inferWireApiForModel", () => { | ||
| it("omits wireApi for Anthropic providers even when the catalog requests one", () => { | ||
| expect(inferWireApiForModel("anthropic", "claude-opus-5", { wire_api: "responses" })).toBeUndefined(); | ||
| }); | ||
|
|
||
| it("falls back to completions when the catalog value is invalid or absent", () => { | ||
| expect(inferWireApiForModel("openai", "gpt-5.5", { wire_api: "grpc" })).toBe("completions"); | ||
| expect(inferWireApiForModel("openai", "gpt-5.5", null)).toBe("completions"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("resolveCopilotSDKCustomProviderFromReflect", () => { | ||
| it("resolves provider baseUrl and model from port when models_url is absent", () => { | ||
| const reflectData = { | ||
| endpoints: [{ provider: "copilot", port: 10002, configured: true, models: ["gpt-5.4", "claude-sonnet-4.6"] }], | ||
| }; | ||
| expect(resolveCopilotSDKCustomProviderFromReflect({ reflectData })).toEqual({ | ||
| model: "gpt-5.4", | ||
| provider: { type: "openai", baseUrl: "http://api-proxy:10002", wireApi: "completions" }, | ||
| }); | ||
| }); | ||
|
|
||
|
|
@@ -439,7 +507,7 @@ describe("awf_reflect.cjs", () => { | |
| }; | ||
| expect(resolveCopilotSDKCustomProviderFromReflect({ reflectData })).toEqual({ | ||
| model: "gpt-4o", | ||
| provider: { type: "openai", baseUrl: "http://172.30.0.30:10002", wireApi: "completions" }, | ||
| }); | ||
| }); | ||
|
|
||
|
|
@@ -469,7 +537,7 @@ describe("awf_reflect.cjs", () => { | |
| }; | ||
| expect(resolveCopilotSDKCustomProviderFromReflect({ reflectData, model: "gpt-5.4" })).toEqual({ | ||
| model: "gpt-5.4", | ||
| provider: { type: "openai", baseUrl: "http://api-proxy:10002", wireApi: "completions" }, | ||
| }); | ||
| }); | ||
|
|
||
|
|
@@ -484,7 +552,53 @@ describe("awf_reflect.cjs", () => { | |
| }; | ||
| expect(resolveCopilotSDKCustomProviderFromReflect({ reflectData, modelsJson })).toEqual({ | ||
| model: "raptor-mini", | ||
| provider: { type: "openai", baseUrl: "http://api-proxy:10002", wireApi: "completions" }, | ||
| }); | ||
| }); | ||
|
|
||
| it("uses wire_api from modelsJson when provided", () => { | ||
|
Contributor
There was a problem hiding this comment. [/tdd] Only one new test was added for Missing coverage:
💡 Suggested test structure (export `inferWireApiForModel` or test via the resolver)describe("inferWireApiForModel (via resolver)", () => {
it("returns completions for openai model with no catalog entry", () => {
const result = resolveCopilotSDKCustomProviderFromReflect({
reflectData: { endpoints: [{ provider: "copilot", port: 10002, configured: true, models: ["new-model"] }] },
model: "new-model",
modelsJson: null,
});
expect(result.provider.wireApi).toBe("completions");
});
it("ignores unrecognized wire_api value and falls back to heuristic", () => {
const modelsJson = {
providers: { "github-copilot": { models: { "gpt-x": { provider_type: "openai", wire_api: "grpc", cost: {} } } } },
};
const result = resolveCopilotSDKCustomProviderFromReflect({
reflectData: { endpoints: [{ provider: "copilot", port: 10002, configured: true, models: ["gpt-x"] }] },
model: "gpt-x",
modelsJson,
});
expect(result.provider.wireApi).toBe("completions"); // falls through to heuristic
});
});@copilot please address this. |
||
| const reflectData = { | ||
| endpoints: [{ provider: "copilot", port: 10002, configured: true, models: ["gpt-5.5"] }], | ||
| }; | ||
| const modelsJson = { | ||
| providers: { | ||
| "github-copilot": { models: { "gpt-5.5": { provider_type: "openai", wire_api: "responses", cost: {} } } }, | ||
| }, | ||
| }; | ||
| expect(resolveCopilotSDKCustomProviderFromReflect({ reflectData, model: "gpt-5.5", modelsJson })).toEqual({ | ||
| model: "gpt-5.5", | ||
| provider: { type: "openai", baseUrl: "http://api-proxy:10002", wireApi: "responses" }, | ||
|
Contributor
There was a problem hiding this comment. Missing test: Anthropic model with explicit catalog 💡 Suggested test to addit("omits wireApi for Anthropic models even when catalog has wire_api", () => {
const reflectData = {
endpoints: [{ provider: "anthropic", port: 10002, configured: true, models: ["claude-opus-5"] }],
};
const modelsJson = {
providers: {
"github-copilot": {
models: { "claude-opus-5": { provider_type: "anthropic", wire_api: "completions", cost: {} } },
},
},
};
const result = resolveCopilotSDKCustomProviderFromReflect({
reflectData,
model: "claude-opus-5",
modelsJson,
});
expect(result?.provider.wireApi).toBeUndefined();
}); |
||
| }); | ||
| }); | ||
|
Contributor
There was a problem hiding this comment. [/tdd] Edge cases worth pinning:
Direct tests here would also guard against accidental breakage if @copilot please address this. |
||
|
|
||
| it("prefers github-copilot catalog metadata when duplicate model names exist across providers", () => { | ||
| const reflectData = { | ||
| endpoints: [{ provider: "copilot", port: 10002, configured: true, models: ["gpt-5.5"] }], | ||
| }; | ||
| const modelsJson = { | ||
| providers: { | ||
| openai: { models: { "gpt-5.5": { provider_type: "openai", cost: {} } } }, | ||
| "github-copilot": { models: { "gpt-5.5": { provider_type: "openai", wire_api: "responses", cost: {} } } }, | ||
| }, | ||
| }; | ||
| expect(resolveCopilotSDKCustomProviderFromReflect({ reflectData, model: "gpt-5.5", modelsJson })).toEqual({ | ||
| model: "gpt-5.5", | ||
| provider: { type: "openai", baseUrl: "http://api-proxy:10002", wireApi: "responses" }, | ||
| }); | ||
| }); | ||
|
|
||
| it("omits wireApi for Anthropic models even when the catalog has wire_api", () => { | ||
| const reflectData = { | ||
| endpoints: [{ provider: "anthropic", port: 10001, configured: true, models: ["claude-opus-5"] }], | ||
| }; | ||
| const modelsJson = { | ||
| providers: { | ||
| "github-copilot": { models: { "claude-opus-5": { provider_type: "anthropic", wire_api: "responses", cost: {} } } }, | ||
| }, | ||
| }; | ||
| expect(resolveCopilotSDKCustomProviderFromReflect({ reflectData, model: "claude-opus-5", modelsJson })).toEqual({ | ||
| model: "claude-opus-5", | ||
| provider: { type: "anthropic", baseUrl: "http://api-proxy:10001" }, | ||
| }); | ||
| }); | ||
|
|
||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.