Skip to content
1 change: 1 addition & 0 deletions app/config/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export const getBuildConfig = () => {
buildMode,
isApp,
template: process.env.DEFAULT_INPUT_TEMPLATE ?? DEFAULT_INPUT_TEMPLATE,
visionModels: process.env.VISION_MODELS || "",
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Environment variable name mismatch with PR objectives.

The PR objectives mention NEXT_PUBLIC_VISION_MODELS, but the code uses VISION_MODELS. This inconsistency should be resolved.

-    visionModels: process.env.VISION_MODELS || "",
+    visionModels: process.env.NEXT_PUBLIC_VISION_MODELS || "",
πŸ“ 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
visionModels: process.env.VISION_MODELS || "",
visionModels: process.env.NEXT_PUBLIC_VISION_MODELS || "",

};
};

Expand Down
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
32 changes: 11 additions & 21 deletions app/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ 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";
import { getClientConfig } from "./config/client";

export function trimTopic(topic: string) {
// Fix an issue where double quotes still show in the Indonesian language
Expand Down Expand Up @@ -252,28 +254,16 @@ 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 clientConfig = getClientConfig();
const envVisionModels = clientConfig.visionModels
?.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 VISION_MODELS env var", () => {
process.env.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 VISION_MODELS", () => {
process.env.VISION_MODELS = "";
expect(isVisionModel("unrelated-model")).toBe(false);

delete process.env.VISION_MODELS;
expect(isVisionModel("unrelated-model")).toBe(false);
expect(isVisionModel("gpt-4-vision")).toBe(true);
});
});
Comment on lines +1 to +67
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

Add missing edge case tests.

Consider adding tests for the following edge cases:

  1. Model names with special characters
  2. Case sensitivity handling
  3. Empty string input
  4. Null/undefined input
+  test.each([
+    ["", "empty string"],
+    [null, "null input"],
+    [undefined, "undefined input"],
+    ["GPT-4-VISION", "uppercase input"],
+    ["gpt-4-vision!", "special characters"],
+  ])("should handle edge case: %s (%s)", (input, description) => {
+    expect(() => isVisionModel(input)).not.toThrow();
+  });

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

🧰 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)

Loading