Skip to content

Commit e5737f5

Browse files
committed
fix: tighten embeddingModelsMatch to token-boundary containment (D2)
The substitution guard's match test was a plain substring check (a.includes(b) || b.includes(a)). That's loose in both directions: a broadly- configured model id like `qwen3-embedding` would MATCH an unrelated served model that merely contains it as an interior token — e.g. served `text-embedding-qwen3-embedding-0.6b` → store 0.6b vectors under the broad identity (wrong-dimension corruption, the exact failure the guard exists to stop). Now the shorter name must align on a `-`/`/` boundary as a genuine PREFIX (vendor- trim: `openai/X` ↔ `X`) or SUFFIX (version-expansion: `…-small` → `…-small-v1`) of the longer — never an interior fragment. Keeps the legitimate tolerance, closes the corruption hole. +6 unit tests (incl. the interior-token rejection and a non-boundary prefix collision). Pi inherits via core. Gate: plugin embedding tests 26/0, Pi tsc clean.
1 parent 214c436 commit e5737f5

2 files changed

Lines changed: 61 additions & 7 deletions

File tree

packages/plugin/src/features/magic-context/memory/embedding-openai.test.ts

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,43 @@
11
import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test";
2-
import { OpenAICompatibleEmbeddingProvider } from "./embedding-openai";
2+
import { embeddingModelsMatch, OpenAICompatibleEmbeddingProvider } from "./embedding-openai";
3+
4+
describe("embeddingModelsMatch token-boundary semantics", () => {
5+
test("exact match", () => {
6+
expect(embeddingModelsMatch("qwen3-embedding-4b", "qwen3-embedding-4b")).toBe(true);
7+
});
8+
test("version-expansion suffix on a boundary matches", () => {
9+
expect(embeddingModelsMatch("text-embedding-3-small-v1", "text-embedding-3-small")).toBe(
10+
true,
11+
);
12+
});
13+
test("vendor-prefix trim on a boundary matches (either direction)", () => {
14+
expect(
15+
embeddingModelsMatch("openai/text-embedding-3-small", "text-embedding-3-small"),
16+
).toBe(true);
17+
expect(
18+
embeddingModelsMatch("text-embedding-3-small", "openai/text-embedding-3-small"),
19+
).toBe(true);
20+
});
21+
test("REJECTS a broad configured name contained as an interior token (corruption hole)", () => {
22+
// The bug: served `…-qwen3-embedding-0.6b` contains configured `qwen3-embedding`
23+
// but `0.6b` is a distinct model token, not a version suffix.
24+
expect(embeddingModelsMatch("text-embedding-qwen3-embedding-0.6b", "qwen3-embedding")).toBe(
25+
false,
26+
);
27+
expect(
28+
embeddingModelsMatch("qwen3-embedding-4b-dwq", "text-embedding-qwen3-embedding-0.6b"),
29+
).toBe(false);
30+
});
31+
test("REJECTS a non-boundary prefix collision (small vs smaller)", () => {
32+
expect(embeddingModelsMatch("text-embedding-3-smallish", "text-embedding-3-small")).toBe(
33+
false,
34+
);
35+
});
36+
test("empty served or requested cannot be compared → not rejected", () => {
37+
expect(embeddingModelsMatch("", "anything")).toBe(true);
38+
expect(embeddingModelsMatch("anything", "")).toBe(true);
39+
});
40+
});
341

442
type FetchLike = typeof fetch;
543

packages/plugin/src/features/magic-context/memory/embedding-openai.ts

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,17 +35,33 @@ function normalizeEndpoint(endpoint?: string): string {
3535
/**
3636
* Whether the model an endpoint served is the model we asked for.
3737
*
38-
* Exact match after trim+lowercase, with prefix/suffix tolerance so a server
39-
* that version-expands a name (`text-embedding-3-small` → `…-small-v1`) or
40-
* trims a vendor prefix still counts as a match. A genuine substitution to a
41-
* DIFFERENT model (e.g. requested `qwen3-embedding-4b-dwq`, served
42-
* `text-embedding-qwen3-embedding-0.6b` — neither contains the other) does not.
38+
* Exact match after trim+lowercase, with TOKEN-BOUNDARY prefix/suffix tolerance
39+
* so a server that version-expands a name (`text-embedding-3-small` →
40+
* `…-small-v1`) or trims a vendor prefix (`openai/text-embedding-3-small` →
41+
* `text-embedding-3-small`) still counts as a match.
42+
*
43+
* Crucially this is NOT a plain substring test. A loose `a.includes(b)` would
44+
* MATCH a broadly-configured name against an unrelated served model that merely
45+
* contains it as a middle token — e.g. configured `qwen3-embedding`, served
46+
* `text-embedding-qwen3-embedding-0.6b` → store 0.6b vectors under the broad
47+
* identity (wrong-dim corruption, the exact failure this guard exists to stop).
48+
* So the shorter name must align on a `-`/`/` boundary as a genuine PREFIX or
49+
* SUFFIX of the longer, never as an interior fragment.
4350
*/
4451
export function embeddingModelsMatch(served: string, requested: string): boolean {
4552
const a = served.trim().toLowerCase();
4653
const b = requested.trim().toLowerCase();
4754
if (a.length === 0 || b.length === 0) return true; // can't compare → don't reject
48-
return a === b || a.includes(b) || b.includes(a);
55+
if (a === b) return true;
56+
const longer = a.length >= b.length ? a : b;
57+
const shorter = a.length >= b.length ? b : a;
58+
const isBoundary = (ch: string) => ch === "-" || ch === "/";
59+
// Version-expansion: longer = shorter + boundary + suffix (e.g. `…-small` → `…-small-v1`).
60+
if (longer.startsWith(shorter) && isBoundary(longer.charAt(shorter.length))) return true;
61+
// Vendor-prefix trim: longer = prefix + boundary + shorter (e.g. `openai/X` ↔ `X`).
62+
if (longer.endsWith(shorter) && isBoundary(longer.charAt(longer.length - shorter.length - 1)))
63+
return true;
64+
return false;
4965
}
5066

5167
/**

0 commit comments

Comments
 (0)