mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 10:10:45 +00:00
perf(catalog): cache manifest built-in model suppression resolver (#74236)
* perf(catalog): cache manifest built-in model suppression resolver * fix(catalog): address PR review comments for manifest suppression resolver * fix(catalog): preserve cached suppression semantics --------- Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
This commit is contained in:
@@ -19,6 +19,11 @@ vi.mock("./model-suppression.runtime.js", () => ({
|
||||
params.provider === "azure-openai-responses" ||
|
||||
params.provider === "openai-codex") &&
|
||||
params.id === "gpt-5.3-codex-spark",
|
||||
buildShouldSuppressBuiltInModel: () => (params: { provider?: string; id?: string }) =>
|
||||
(params.provider === "openai" ||
|
||||
params.provider === "azure-openai-responses" ||
|
||||
params.provider === "openai-codex") &&
|
||||
params.id === "gpt-5.3-codex-spark",
|
||||
}));
|
||||
|
||||
function mockCatalogImportFailThenRecover() {
|
||||
|
||||
@@ -149,7 +149,7 @@ export async function loadModelCatalog(params?: {
|
||||
const piSdk = await importPiSdk();
|
||||
logStage("pi-sdk-imported");
|
||||
const agentDir = resolveOpenClawAgentDir();
|
||||
const { shouldSuppressBuiltInModel } = await loadModelSuppression();
|
||||
const { buildShouldSuppressBuiltInModel } = await loadModelSuppression();
|
||||
logStage("catalog-deps-ready");
|
||||
const authStorage = piSdk.discoverAuthStorage(
|
||||
agentDir,
|
||||
@@ -164,6 +164,10 @@ export async function loadModelCatalog(params?: {
|
||||
logStage("registry-ready");
|
||||
const entries = Array.isArray(registry) ? registry : registry.getAll();
|
||||
logStage("registry-read", `entries=${entries.length}`);
|
||||
|
||||
const shouldSuppressBuiltInModel = buildShouldSuppressBuiltInModel({ config: cfg });
|
||||
logStage("suppress-resolver-ready");
|
||||
|
||||
for (const entry of entries) {
|
||||
const id = normalizeOptionalString(entry?.id) ?? "";
|
||||
if (!id) {
|
||||
@@ -173,7 +177,7 @@ export async function loadModelCatalog(params?: {
|
||||
if (!provider) {
|
||||
continue;
|
||||
}
|
||||
if (shouldSuppressBuiltInModel({ provider, id, config: cfg })) {
|
||||
if (shouldSuppressBuiltInModel({ provider, id })) {
|
||||
continue;
|
||||
}
|
||||
const name = normalizeOptionalString(entry?.name ?? id) || id;
|
||||
|
||||
@@ -1,10 +1,21 @@
|
||||
import { shouldSuppressBuiltInModel as shouldSuppressBuiltInModelImpl } from "./model-suppression.js";
|
||||
import {
|
||||
buildShouldSuppressBuiltInModel as buildShouldSuppressBuiltInModelImpl,
|
||||
shouldSuppressBuiltInModel as shouldSuppressBuiltInModelImpl,
|
||||
} from "./model-suppression.js";
|
||||
|
||||
type ShouldSuppressBuiltInModel =
|
||||
typeof import("./model-suppression.js").shouldSuppressBuiltInModel;
|
||||
type BuildShouldSuppressBuiltInModel =
|
||||
typeof import("./model-suppression.js").buildShouldSuppressBuiltInModel;
|
||||
|
||||
export function shouldSuppressBuiltInModel(
|
||||
...args: Parameters<ShouldSuppressBuiltInModel>
|
||||
): ReturnType<ShouldSuppressBuiltInModel> {
|
||||
return shouldSuppressBuiltInModelImpl(...args);
|
||||
}
|
||||
|
||||
export function buildShouldSuppressBuiltInModel(
|
||||
...args: Parameters<BuildShouldSuppressBuiltInModel>
|
||||
): ReturnType<BuildShouldSuppressBuiltInModel> {
|
||||
return buildShouldSuppressBuiltInModelImpl(...args);
|
||||
}
|
||||
|
||||
@@ -1,17 +1,23 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
buildManifestBuiltInModelSuppressionResolver: vi.fn(),
|
||||
resolveManifestBuiltInModelSuppression: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("../plugins/manifest-model-suppression.js", () => ({
|
||||
buildManifestBuiltInModelSuppressionResolver: mocks.buildManifestBuiltInModelSuppressionResolver,
|
||||
resolveManifestBuiltInModelSuppression: mocks.resolveManifestBuiltInModelSuppression,
|
||||
}));
|
||||
|
||||
import { shouldSuppressBuiltInModel } from "./model-suppression.js";
|
||||
import {
|
||||
buildShouldSuppressBuiltInModel,
|
||||
shouldSuppressBuiltInModel,
|
||||
} from "./model-suppression.js";
|
||||
|
||||
describe("model suppression", () => {
|
||||
beforeEach(() => {
|
||||
mocks.buildManifestBuiltInModelSuppressionResolver.mockReset();
|
||||
mocks.resolveManifestBuiltInModelSuppression.mockReset();
|
||||
});
|
||||
|
||||
@@ -43,4 +49,48 @@ describe("model suppression", () => {
|
||||
|
||||
expect(mocks.resolveManifestBuiltInModelSuppression).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
describe("buildShouldSuppressBuiltInModel", () => {
|
||||
beforeEach(() => {
|
||||
mocks.buildManifestBuiltInModelSuppressionResolver.mockReset();
|
||||
});
|
||||
|
||||
it("creates a reusable manifest resolver with normalized provider and model ids", () => {
|
||||
const resolver = vi
|
||||
.fn()
|
||||
.mockReturnValueOnce({ suppress: true, errorMessage: "manifest suppression" })
|
||||
.mockReturnValueOnce(undefined);
|
||||
const config = {};
|
||||
mocks.buildManifestBuiltInModelSuppressionResolver.mockReturnValueOnce(resolver);
|
||||
|
||||
const shouldSuppress = buildShouldSuppressBuiltInModel({ config });
|
||||
|
||||
expect(shouldSuppress({ provider: "bedrock", id: "Claude-3" })).toBe(true);
|
||||
expect(shouldSuppress({ provider: "aws-bedrock", id: "claude-4" })).toBe(false);
|
||||
expect(mocks.buildManifestBuiltInModelSuppressionResolver).toHaveBeenCalledOnce();
|
||||
expect(mocks.buildManifestBuiltInModelSuppressionResolver).toHaveBeenCalledWith({
|
||||
config,
|
||||
env: process.env,
|
||||
});
|
||||
expect(resolver).toHaveBeenNthCalledWith(1, {
|
||||
provider: "amazon-bedrock",
|
||||
id: "claude-3",
|
||||
});
|
||||
expect(resolver).toHaveBeenNthCalledWith(2, {
|
||||
provider: "amazon-bedrock",
|
||||
id: "claude-4",
|
||||
});
|
||||
});
|
||||
|
||||
it("does not call the manifest resolver for empty provider or model ids", () => {
|
||||
const resolver = vi.fn();
|
||||
mocks.buildManifestBuiltInModelSuppressionResolver.mockReturnValueOnce(resolver);
|
||||
|
||||
const shouldSuppress = buildShouldSuppressBuiltInModel({});
|
||||
|
||||
expect(shouldSuppress({ provider: "openai", id: "" })).toBe(false);
|
||||
expect(shouldSuppress({ provider: "", id: "gpt-5.5" })).toBe(false);
|
||||
expect(resolver).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,5 +1,8 @@
|
||||
import type { OpenClawConfig } from "../config/types.openclaw.js";
|
||||
import { resolveManifestBuiltInModelSuppression } from "../plugins/manifest-model-suppression.js";
|
||||
import {
|
||||
buildManifestBuiltInModelSuppressionResolver,
|
||||
resolveManifestBuiltInModelSuppression,
|
||||
} from "../plugins/manifest-model-suppression.js";
|
||||
import { normalizeLowercaseStringOrEmpty } from "../shared/string-coerce.js";
|
||||
import { normalizeProviderId } from "./provider-id.js";
|
||||
|
||||
@@ -66,3 +69,21 @@ export function buildSuppressedBuiltInModelError(params: {
|
||||
}): string | undefined {
|
||||
return resolveBuiltInModelSuppression(params)?.errorMessage;
|
||||
}
|
||||
|
||||
export function buildShouldSuppressBuiltInModel(params: {
|
||||
config?: OpenClawConfig;
|
||||
}): (input: { provider?: string | null; id?: string | null; baseUrl?: string | null }) => boolean {
|
||||
const resolver = buildManifestBuiltInModelSuppressionResolver({
|
||||
config: params.config,
|
||||
env: process.env,
|
||||
});
|
||||
|
||||
return (input) => {
|
||||
const provider = normalizeProviderId(input.provider ?? "");
|
||||
const id = normalizeLowercaseStringOrEmpty(input.id);
|
||||
if (!provider || !id) {
|
||||
return false;
|
||||
}
|
||||
return resolver({ ...input, provider, id })?.suppress ?? false;
|
||||
};
|
||||
}
|
||||
|
||||
@@ -9,6 +9,7 @@ vi.mock("./plugin-registry.js", () => ({
|
||||
}));
|
||||
|
||||
import {
|
||||
buildManifestBuiltInModelSuppressionResolver,
|
||||
clearManifestModelSuppressionCacheForTest,
|
||||
resolveManifestBuiltInModelSuppression,
|
||||
} from "./manifest-model-suppression.js";
|
||||
@@ -46,6 +47,30 @@ describe("manifest model suppression", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("buildManifestBuiltInModelSuppressionResolver", () => {
|
||||
it("reads planned manifest suppressions once per resolver creation", () => {
|
||||
const config = { plugins: { entries: { openai: { enabled: true } } } };
|
||||
|
||||
const resolver = buildManifestBuiltInModelSuppressionResolver({
|
||||
config,
|
||||
env: process.env,
|
||||
});
|
||||
|
||||
expect(mocks.loadPluginManifestRegistryForPluginRegistry).toHaveBeenCalledTimes(1);
|
||||
|
||||
resolver({
|
||||
provider: "azure-openai-responses",
|
||||
id: "gpt-5.3-codex-spark",
|
||||
});
|
||||
resolver({
|
||||
provider: "azure-openai-responses",
|
||||
id: "gpt-5.3-codex-spark",
|
||||
});
|
||||
|
||||
expect(mocks.loadPluginManifestRegistryForPluginRegistry).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
it("resolves manifest suppressions for declared provider aliases", () => {
|
||||
expect(
|
||||
resolveManifestBuiltInModelSuppression({
|
||||
@@ -89,6 +114,29 @@ describe("manifest model suppression", () => {
|
||||
expect(mocks.loadPluginManifestRegistryForPluginRegistry).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("reuses planned manifest suppressions inside a resolver instance", () => {
|
||||
const config = { plugins: { entries: { openai: { enabled: true } } } };
|
||||
|
||||
const resolver = buildManifestBuiltInModelSuppressionResolver({
|
||||
config,
|
||||
env: process.env,
|
||||
});
|
||||
|
||||
expect(
|
||||
resolver({
|
||||
provider: "azure-openai-responses",
|
||||
id: "gpt-5.3-codex-spark",
|
||||
})?.suppress,
|
||||
).toBe(true);
|
||||
expect(
|
||||
resolver({
|
||||
provider: "azure-openai-responses",
|
||||
id: "gpt-4.1",
|
||||
}),
|
||||
).toBeUndefined();
|
||||
expect(mocks.loadPluginManifestRegistryForPluginRegistry).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("matches conditional suppressions by base URL host", () => {
|
||||
mocks.loadPluginManifestRegistryForPluginRegistry.mockReturnValue({
|
||||
diagnostics: [],
|
||||
|
||||
@@ -103,6 +103,59 @@ export function clearManifestModelSuppressionCacheForTest(): void {
|
||||
// Manifest suppressions are read fresh. Keep the test hook as a no-op.
|
||||
}
|
||||
|
||||
export function buildManifestBuiltInModelSuppressionResolver(params: {
|
||||
config?: OpenClawConfig;
|
||||
workspaceDir?: string;
|
||||
env?: NodeJS.ProcessEnv;
|
||||
}) {
|
||||
const suppressions = listManifestModelCatalogSuppressions({
|
||||
config: params.config,
|
||||
workspaceDir: params.workspaceDir,
|
||||
env: params.env ?? process.env,
|
||||
});
|
||||
|
||||
return (input: {
|
||||
provider?: string | null;
|
||||
id?: string | null;
|
||||
baseUrl?: string | null;
|
||||
}) => {
|
||||
const provider = normalizeLowercaseStringOrEmpty(input.provider);
|
||||
const modelId = normalizeLowercaseStringOrEmpty(input.id);
|
||||
if (!provider || !modelId) {
|
||||
return undefined;
|
||||
}
|
||||
const mergeKey = buildModelCatalogMergeKey(provider, modelId);
|
||||
const suppression = suppressions.find(
|
||||
(entry) =>
|
||||
entry.mergeKey === mergeKey &&
|
||||
manifestSuppressionMatchesConditions({
|
||||
suppression: entry,
|
||||
provider,
|
||||
baseUrl: input.baseUrl,
|
||||
config: params.config,
|
||||
}),
|
||||
);
|
||||
if (!suppression) {
|
||||
return undefined;
|
||||
}
|
||||
return {
|
||||
suppress: true,
|
||||
errorMessage: buildManifestSuppressionError({
|
||||
provider,
|
||||
modelId,
|
||||
reason: suppression.reason,
|
||||
}),
|
||||
};
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves whether a built-in model should be suppressed based on manifest declarations.
|
||||
*
|
||||
* Note: This function instantiates a fresh resolver on every call, which incurs a full
|
||||
* filesystem scan of the manifest registry. For hot paths (like building the model catalog),
|
||||
* instantiate and reuse `buildManifestBuiltInModelSuppressionResolver` instead.
|
||||
*/
|
||||
export function resolveManifestBuiltInModelSuppression(params: {
|
||||
provider?: string | null;
|
||||
id?: string | null;
|
||||
@@ -111,35 +164,14 @@ export function resolveManifestBuiltInModelSuppression(params: {
|
||||
env?: NodeJS.ProcessEnv;
|
||||
baseUrl?: string | null;
|
||||
}) {
|
||||
const provider = normalizeLowercaseStringOrEmpty(params.provider);
|
||||
const modelId = normalizeLowercaseStringOrEmpty(params.id);
|
||||
if (!provider || !modelId) {
|
||||
return undefined;
|
||||
}
|
||||
const mergeKey = buildModelCatalogMergeKey(provider, modelId);
|
||||
const suppression = listManifestModelCatalogSuppressions({
|
||||
const resolver = buildManifestBuiltInModelSuppressionResolver({
|
||||
config: params.config,
|
||||
workspaceDir: params.workspaceDir,
|
||||
env: params.env ?? process.env,
|
||||
}).find(
|
||||
(entry) =>
|
||||
entry.mergeKey === mergeKey &&
|
||||
manifestSuppressionMatchesConditions({
|
||||
suppression: entry,
|
||||
provider,
|
||||
baseUrl: params.baseUrl,
|
||||
config: params.config,
|
||||
}),
|
||||
);
|
||||
if (!suppression) {
|
||||
return undefined;
|
||||
}
|
||||
return {
|
||||
suppress: true,
|
||||
errorMessage: buildManifestSuppressionError({
|
||||
provider,
|
||||
modelId,
|
||||
reason: suppression.reason,
|
||||
}),
|
||||
};
|
||||
env: params.env,
|
||||
});
|
||||
return resolver({
|
||||
provider: params.provider,
|
||||
id: params.id,
|
||||
baseUrl: params.baseUrl,
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user