Skip to content

Commit a67a614

Browse files
thaolaptrinhclaude
andcommitted
refactor(catalog): review cleanup — drop dead fetchModelList, dedupe helpers
Post-review of PR #8: - remove fetchModelList (no remaining callers after server.ts migrated to refreshCatalog) - extract displayNameFor() to collapse the three display-name fallback chains in catalog.ts - share the URL-filtering CC models fetch mock between test files via tests/helpers.ts Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 3c4fe29 commit a67a614

5 files changed

Lines changed: 36 additions & 43 deletions

File tree

‎src/translate/catalog.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,16 @@ function buildCatalog(models: CatalogModel[]): ModelCatalog {
4545
};
4646
}
4747

48+
/** Best display name: API name → static name → bare last path segment. */
49+
function displayNameFor(id: string, apiName?: string): string {
50+
return apiName ?? STATIC_NAMES[id] ?? id.split("/").pop() ?? id;
51+
}
52+
4853
export function getStaticCatalog(): ModelCatalog {
4954
return buildCatalog(
5055
STATIC_BUILTIN.map((id) => ({
5156
id,
52-
displayName: STATIC_NAMES[id] ?? id.split("/").pop() ?? id,
57+
displayName: displayNameFor(id),
5358
contextWindow: STATIC_CONTEXT[id] ?? 128_000,
5459
source: "static" as const,
5560
})),
@@ -120,7 +125,7 @@ export async function refreshCatalog(
120125
const api = apiById.get(id);
121126
merged.push({
122127
id,
123-
displayName: api?.name ?? STATIC_NAMES[id] ?? id.split("/").pop() ?? id,
128+
displayName: displayNameFor(id, api?.name),
124129
contextWindow: api?.context_length ?? STATIC_CONTEXT[id] ?? 128_000,
125130
source: api ? "api" : "static",
126131
});
@@ -130,7 +135,7 @@ export async function refreshCatalog(
130135
if (seen.has(m.id)) continue;
131136
merged.push({
132137
id: m.id,
133-
displayName: m.name ?? m.id.split("/").pop() ?? m.id,
138+
displayName: displayNameFor(m.id, m.name),
134139
contextWindow: m.context_length ?? 128_000,
135140
source: "api",
136141
});

‎src/translate/models.ts‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Model resolution, aliasing, and discovery against the CC provider API.
22

33
import modelsData from "@/models.json" with { type: "json" };
4-
import { getCatalog, refreshCatalog } from "@/translate/catalog.js";
4+
import { getCatalog } from "@/translate/catalog.js";
55

66
const BUILTIN_MODELS: string[] = modelsData.builtin;
77
const SHORT_ALIASES: Record<string, string> = modelsData.shortAliases;
@@ -11,16 +11,6 @@ const REASONING_EFFORTS: Record<string, string[]> = modelsData.reasoningEfforts
1111
/** Rank ordering of effort levels (low → max). Used to clip to the nearest valid. */
1212
const EFFORT_RANK: Record<string, number> = { low: 0, medium: 1, high: 2, xhigh: 3, max: 4 };
1313

14-
/**
15-
* Fetch available open-source models from the CC provider API (closed models
16-
* are filtered out) and merge them into the catalog. Returns the merged model
17-
* list. Failures keep the current (static fallback) catalog.
18-
*/
19-
export async function fetchModelList(apiBase: string, apiKey: string): Promise<string[]> {
20-
const catalog = await refreshCatalog(apiBase, apiKey);
21-
return catalog.ids;
22-
}
23-
2414
export function getDefaultModels(): string[] {
2515
return getCatalog().ids;
2616
}

‎tests/catalog.test.ts‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,24 +9,12 @@ import {
99
import { resolveModel } from "@/translate/models.js";
1010
import { loadConfig } from "@/config.js";
1111
import { createServer } from "@/server.js";
12+
import { mockCcModelsFetch } from "./helpers.js";
1213

1314
const API_BASE = "https://api.test-cc.example";
1415
const API_KEY = "test-key";
1516

16-
function mockFetch(models: unknown, calls: { count: number } = { count: 0 }) {
17-
const realFetch = globalThis.fetch.bind(globalThis);
18-
return vi.spyOn(globalThis, "fetch").mockImplementation(async (input, init) => {
19-
const url = typeof input === "string" ? input : (input as Request).url ?? String(input);
20-
// Only intercept CC API calls; other fetches (e.g. hitting the test server
21-
// itself) must go to the real network stack.
22-
if (!url.includes("/provider/v1/models")) return realFetch(input, init);
23-
calls.count += 1;
24-
return new Response(JSON.stringify({ object: "list", data: models }), {
25-
status: 200,
26-
headers: { "Content-Type": "application/json" },
27-
}) as unknown as Response;
28-
});
29-
}
17+
const mockFetch = mockCcModelsFetch;
3018

3119
const API_MODELS = [
3220
{ id: "claude-opus-5", name: "Claude Opus 5", context_length: 1000000 },

‎tests/helpers.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import { vi } from "vitest";
2+
3+
/**
4+
* Mock only the CC provider models endpoint. Other fetches (e.g. requests to
5+
* the test server itself) must reach the real network stack — a blanket fetch
6+
* mock silently swallows them and the test reads the mock payload instead of
7+
* the server's actual response.
8+
*/
9+
export function mockCcModelsFetch(models: unknown, calls: { count: number } = { count: 0 }) {
10+
const realFetch = globalThis.fetch.bind(globalThis);
11+
return vi.spyOn(globalThis, "fetch").mockImplementation(async (input, init) => {
12+
const url = typeof input === "string" ? input : (input as Request).url ?? String(input);
13+
if (!url.includes("/provider/v1/models")) return realFetch(input, init);
14+
calls.count += 1;
15+
return new Response(JSON.stringify({ object: "list", data: models }), {
16+
status: 200,
17+
headers: { "Content-Type": "application/json" },
18+
}) as unknown as Response;
19+
});
20+
}

‎tests/setup-opencode.test.ts‎

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { describe, test, expect, beforeEach, afterEach, vi } from "vitest";
22
import { setupOpenCodeConfig } from "@/setup/opencode.js";
33
import { __resetCatalogForTests } from "@/translate/catalog.js";
4+
import { mockCcModelsFetch } from "./helpers.js";
45
import fs from "node:fs";
56

67
// Never hit the real CC API or read the developer's saved auth during tests.
@@ -64,21 +65,10 @@ describe("setupOpenCodeConfig", () => {
6465
test("builds config from the live API catalog when a key is available", async () => {
6566
process.env.CC_API_KEY = "test-key";
6667
process.env.CC_API_BASE = "https://api.test-cc.example";
67-
const realFetch = globalThis.fetch.bind(globalThis);
68-
vi.spyOn(globalThis, "fetch").mockImplementation(async (input, init) => {
69-
const url = typeof input === "string" ? input : (input as Request).url ?? String(input);
70-
if (!url.includes("/provider/v1/models")) return realFetch(input, init);
71-
return new Response(
72-
JSON.stringify({
73-
object: "list",
74-
data: [
75-
{ id: "claude-opus-5", name: "Claude Opus 5", context_length: 1000000 },
76-
{ id: "zai-org/GLM-5.3", name: "GLM-5.3", context_length: 1000000 },
77-
],
78-
}),
79-
{ status: 200 },
80-
) as unknown as Response;
81-
});
68+
mockCcModelsFetch([
69+
{ id: "claude-opus-5", name: "Claude Opus 5", context_length: 1000000 },
70+
{ id: "zai-org/GLM-5.3", name: "GLM-5.3", context_length: 1000000 },
71+
]);
8272

8373
await setupOpenCodeConfig("local");
8474
const config = JSON.parse(written as string);

0 commit comments

Comments
 (0)