Skip to content
16 changes: 16 additions & 0 deletions app/constant.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,22 @@ export const DEFAULT_TTS_VOICES = [
"shimmer",
];

export const VISION_MODEL_REGEXES = [
/vision/,
/gpt-4o/,
/claude-3/,
/gemini-1\.5/,
/gemini-exp/,
/gemini-2\.0/,
/learnlm/,
/qwen-vl/,
/qwen2-vl/,
/gpt-4-turbo(?!.*preview)/, // Matches "gpt-4-turbo" but not "gpt-4-turbo-preview"
/^dall-e-3$/, // Matches exactly "dall-e-3"
];

export const EXCLUDE_VISION_MODEL_REGEXES = [/claude-3-5-haiku-20241022/];

const openaiModels = [
"gpt-3.5-turbo",
"gpt-3.5-turbo-1106",
Expand Down
30 changes: 9 additions & 21 deletions app/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { RequestMessage } from "./client/api";
import { ServiceProvider } from "./constant";
// import { fetch as tauriFetch, ResponseType } from "@tauri-apps/api/http";
import { fetch as tauriStreamFetch } from "./utils/stream";
import { VISION_MODEL_REGEXES, EXCLUDE_VISION_MODEL_REGEXES } from "./constant";

export function trimTopic(topic: string) {
// Fix an issue where double quotes still show in the Indonesian language
Expand Down Expand Up @@ -252,28 +253,15 @@ export function getMessageImages(message: RequestMessage): string[] {
}

export function isVisionModel(model: string) {
// Note: This is a better way using the TypeScript feature instead of `&&` or `||` (ts v5.5.0-dev.20240314 I've been using)

const excludeKeywords = ["claude-3-5-haiku-20241022"];
const visionKeywords = [
"vision",
"gpt-4o",
"claude-3",
"gemini-1.5",
"gemini-exp",
"gemini-2.0",
"learnlm",
"qwen-vl",
"qwen2-vl",
];
const isGpt4Turbo =
model.includes("gpt-4-turbo") && !model.includes("preview");

const envVisionModels = process.env.NEXT_PUBLIC_VISION_MODELS?.split(",").map(
(m) => m.trim(),
);
if (envVisionModels?.includes(model)) {
return true;
}
return (
!excludeKeywords.some((keyword) => model.includes(keyword)) &&
(visionKeywords.some((keyword) => model.includes(keyword)) ||
isGpt4Turbo ||
isDalle3(model))
!EXCLUDE_VISION_MODEL_REGEXES.some((regex) => regex.test(model)) &&
VISION_MODEL_REGEXES.some((regex) => regex.test(model))
);
}

Expand Down
67 changes: 67 additions & 0 deletions test/vision-model-checker.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { isVisionModel } from "../app/utils";

describe("isVisionModel", () => {
const originalEnv = process.env;

beforeEach(() => {
jest.resetModules();
process.env = { ...originalEnv };
});

afterEach(() => {
process.env = originalEnv;
});

test("should identify vision models using regex patterns", () => {
const visionModels = [
"gpt-4-vision",
"claude-3-opus",
"gemini-1.5-pro",
"gemini-2.0",
"gemini-exp-vision",
"learnlm-vision",
"qwen-vl-max",
"qwen2-vl-max",
"gpt-4-turbo",
"dall-e-3",
];

visionModels.forEach((model) => {
expect(isVisionModel(model)).toBe(true);
});
});

test("should exclude specific models", () => {
expect(isVisionModel("claude-3-5-haiku-20241022")).toBe(false);
});
Comment on lines +34 to +36
Copy link
Contributor

Choose a reason for hiding this comment

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

πŸ› οΈ Refactor suggestion

Enhance exclusion testing with more patterns and documentation.

The current test is too specific. Consider:

  1. Adding more exclusion patterns
  2. Documenting why certain models are excluded
  3. Using test.each for better readability
-test("should exclude specific models", () => {
-  expect(isVisionModel("claude-3-5-haiku-20241022")).toBe(false);
-});
+test.each([
+  // Models excluded due to specific version constraints
+  ["claude-3-5-haiku-20241022", "Haiku variant without vision capabilities"],
+  // Add more excluded models with reasons
+])("should exclude %s (%s)", (model, reason) => {
+  expect(isVisionModel(model)).toBe(false);
+});

Committable suggestion skipped: line range outside the PR's diff.


test("should not identify non-vision models", () => {
const nonVisionModels = [
"gpt-3.5-turbo",
"gpt-4-turbo-preview",
"claude-2",
"regular-model",
];

nonVisionModels.forEach((model) => {
expect(isVisionModel(model)).toBe(false);
});
});

test("should identify models from NEXT_PUBLIC_VISION_MODELS env var", () => {
process.env.NEXT_PUBLIC_VISION_MODELS = "custom-vision-model,another-vision-model";

expect(isVisionModel("custom-vision-model")).toBe(true);
expect(isVisionModel("another-vision-model")).toBe(true);
expect(isVisionModel("unrelated-model")).toBe(false);
});

test("should handle empty or missing NEXT_PUBLIC_VISION_MODELS", () => {
process.env.NEXT_PUBLIC_VISION_MODELS = "";
expect(isVisionModel("unrelated-model")).toBe(false);

delete process.env.NEXT_PUBLIC_VISION_MODELS;
expect(isVisionModel("unrelated-model")).toBe(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

πŸ› οΈ Refactor suggestion

Replace delete operator with undefined assignment.

The delete operator can impact performance as flagged by static analysis.

-    delete process.env.NEXT_PUBLIC_VISION_MODELS;
-    expect(isVisionModel("unrelated-model")).toBe(false);
+    process.env.NEXT_PUBLIC_VISION_MODELS = undefined;
+    expect(isVisionModel("unrelated-model")).toBe(false);
πŸ“ Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
delete process.env.NEXT_PUBLIC_VISION_MODELS;
expect(isVisionModel("unrelated-model")).toBe(false);
process.env.NEXT_PUBLIC_VISION_MODELS = undefined;
expect(isVisionModel("unrelated-model")).toBe(false);
🧰 Tools
πŸͺ› Biome (1.9.4)

[error] 63-63: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)

expect(isVisionModel("gpt-4-vision")).toBe(true);
});
});