From e5e5e48b0516d69b7ace93c49bf0e731044661a8 Mon Sep 17 00:00:00 2001 From: eastagiletracker <310448263+eastagiletracker@users.noreply.github.com> Date: Tue, 29 Sep 2026 22:12:04 +0700 Subject: [PATCH] fix(grep): send synonym alternations to the embedding daemon patternIsSemanticFriendly counted `|` as a regex metacharacter, so any pattern with two or more alternatives (`data loss|concurrent writer|race condition`) was rejected and grep fell back to lexical-only search, even though a list of synonyms is the case embeddings answer best. Stop counting `|` toward the metachar limit in both the pre-tool-use fast path (grep-direct) and the virtual shell interceptor, and cap the number of alternatives at 8 so a pathological many-clause pattern still goes lexical. Patterns with more than one other metachar are unchanged. --- src/hooks/grep-direct.ts | 12 ++- src/shell/grep-interceptor.ts | 11 ++- .../claude-code/grep-direct-semantic.test.ts | 82 +++++++++++++++++++ tests/claude-code/grep-interceptor.test.ts | 36 ++++++++ 4 files changed, 135 insertions(+), 6 deletions(-) create mode 100644 tests/claude-code/grep-direct-semantic.test.ts diff --git a/src/hooks/grep-direct.ts b/src/hooks/grep-direct.ts index 2c8bb52be..3a57cd1a2 100644 --- a/src/hooks/grep-direct.ts +++ b/src/hooks/grep-direct.ts @@ -33,12 +33,18 @@ function getEmbedClient(): EmbedClient { return sharedEmbedClient; } +const MAX_SEMANTIC_ALTERNATIVES = 8; + function patternIsSemanticFriendly(pattern: string, fixedString: boolean): boolean { if (!pattern || pattern.length < 2) return false; if (fixedString) return true; - const meta = pattern.match(/[|()\[\]{}+?^$\\]/g); - if (!meta) return true; - return meta.length <= 1; + // `|` is not counted as a metacharacter: a synonym alternation like + // `data loss|concurrent writer|race condition` is exactly the kind of + // query embeddings answer best. Cap the number of alternatives so a + // pathological many-clause pattern still falls back to lexical. + const meta = pattern.match(/[()\[\]{}+?^$\\]/g); + if (meta && meta.length > 1) return false; + return pattern.split("|").length <= MAX_SEMANTIC_ALTERNATIVES; } export interface GrepParams { diff --git a/src/shell/grep-interceptor.ts b/src/shell/grep-interceptor.ts index cd69992bd..bf5f197b1 100644 --- a/src/shell/grep-interceptor.ts +++ b/src/shell/grep-interceptor.ts @@ -36,6 +36,8 @@ function getGrepEmbedClient(): EmbedClient { return sharedGrepEmbedClient; } +const MAX_SEMANTIC_ALTERNATIVES = 8; + /** * Plain-text-ish pattern → candidate for semantic search. * Skip regex-heavy queries (many metachars) where cosine similarity is not @@ -45,9 +47,12 @@ function patternIsSemanticFriendly(pattern: string, fixedString: boolean): boole if (!pattern || pattern.length < 2) return false; if (fixedString) return true; // Literal-ish patterns with only occasional `.*` are still fine for semantic. - const metaMatches = pattern.match(/[|()\[\]{}+?^$\\]/g); - if (!metaMatches) return true; - return metaMatches.length <= 1; + // `|` is not counted: synonym alternations (`foo|bar|baz`) are what + // embeddings are good at. Cap the number of alternatives so a + // pathological many-clause pattern still falls back to lexical. + const metaMatches = pattern.match(/[()\[\]{}+?^$\\]/g); + if (metaMatches && metaMatches.length > 1) return false; + return pattern.split("|").length <= MAX_SEMANTIC_ALTERNATIVES; } const MAX_FALLBACK_CANDIDATES = 500; diff --git a/tests/claude-code/grep-direct-semantic.test.ts b/tests/claude-code/grep-direct-semantic.test.ts new file mode 100644 index 000000000..6e00f62c4 --- /dev/null +++ b/tests/claude-code/grep-direct-semantic.test.ts @@ -0,0 +1,82 @@ +import { describe, it, expect, vi, afterEach } from "vitest"; + +// Semantic gating for handleGrepDirect (the pre-tool-use fast path). The +// sibling grep-direct.test.ts pins embed() to null to keep its lexical +// assertions deterministic; this file stubs the embed client with a spy so +// it can assert *whether* a pattern is sent to the daemon at all. +const { mockEmbed } = vi.hoisted(() => ({ mockEmbed: vi.fn() })); +vi.mock("../../src/embeddings/client.js", () => ({ + EmbedClient: class { + async embed(text: string, kind: string) { return mockEmbed(text, kind); } + async warmup() { return false; } + }, +})); +vi.mock("../../src/embeddings/disable.js", () => ({ + embeddingsDisabled: () => false, + embeddingsStatus: () => "enabled", +})); + +import { handleGrepDirect, type GrepParams } from "../../src/hooks/grep-direct.js"; + +describe("handleGrepDirect: semantic pattern gating", () => { + const baseParams: GrepParams = { + pattern: "foo", targetPath: "/", + ignoreCase: false, wordMatch: false, filesOnly: false, countOnly: false, + lineNumber: false, invertMatch: false, fixedString: false, + }; + + function mockApi() { + return { query: vi.fn().mockResolvedValue([]) } as any; + } + + function sqlOf(api: { query: ReturnType }): string { + return api.query.mock.calls.map(c => String(c[0])).join("\n"); + } + + afterEach(() => { mockEmbed.mockReset(); }); + + it("embeds synonym alternations and runs the hybrid query (issue #86)", async () => { + mockEmbed.mockResolvedValue([0.1, 0.2, 0.3]); + const api = mockApi(); + await handleGrepDirect(api, "memory", "sessions", { + ...baseParams, pattern: "silent data loss|concurrent writer|race condition", + }); + expect(mockEmbed).toHaveBeenCalledWith("silent data loss|concurrent writer|race condition", "query"); + expect(sqlOf(api)).toContain("<#>"); + }); + + it("embeds plain patterns", async () => { + mockEmbed.mockResolvedValue([0.1]); + await handleGrepDirect(mockApi(), "memory", "sessions", { ...baseParams, pattern: "deploy failed" }); + expect(mockEmbed).toHaveBeenCalledWith("deploy failed", "query"); + }); + + it("skips embedding for regex-heavy patterns even when they contain `|`", async () => { + mockEmbed.mockResolvedValue([0.1]); + const api = mockApi(); + await handleGrepDirect(api, "memory", "sessions", { ...baseParams, pattern: "(foo|bar)\\+" }); + expect(mockEmbed).not.toHaveBeenCalled(); + expect(sqlOf(api)).not.toContain("<#>"); + }); + + it("skips embedding for alternations with more than 8 alternatives", async () => { + mockEmbed.mockResolvedValue([0.1]); + await handleGrepDirect(mockApi(), "memory", "sessions", { + ...baseParams, pattern: "a1|a2|a3|a4|a5|a6|a7|a8|a9", + }); + expect(mockEmbed).not.toHaveBeenCalled(); + }); + + it("embeds an alternation at the 8-alternative limit", async () => { + mockEmbed.mockResolvedValue([0.1]); + await handleGrepDirect(mockApi(), "memory", "sessions", { + ...baseParams, pattern: "a1|a2|a3|a4|a5|a6|a7|a8", + }); + expect(mockEmbed).toHaveBeenCalled(); + }); + + it("skips embedding for patterns shorter than 2 chars", async () => { + await handleGrepDirect(mockApi(), "memory", "sessions", { ...baseParams, pattern: "|" }); + expect(mockEmbed).not.toHaveBeenCalled(); + }); +}); diff --git a/tests/claude-code/grep-interceptor.test.ts b/tests/claude-code/grep-interceptor.test.ts index 20e9bbe92..f68b2690d 100644 --- a/tests/claude-code/grep-interceptor.test.ts +++ b/tests/claude-code/grep-interceptor.test.ts @@ -417,6 +417,42 @@ describe("grep interceptor", () => { expect(mockEmbed).not.toHaveBeenCalled(); }); + it("embeds synonym alternations like `foo|bar|baz` (issue #86)", async () => { + // `|` separates different surface forms of the same concept, which is + // the case embeddings are for. It must not count toward the + // regex-heavy metachar limit. + mockEmbed.mockResolvedValueOnce([0.4, 0.5, 0.6]); + const client = makeClient([]); + const fs = await DeeplakeFs.create(client as never, "test", "/memory"); + const searchSpy = vi.spyOn(grepCore, "searchDeeplakeTables").mockResolvedValue([]); + + const cmd = createGrepCommand(client as never, fs, "test", "sessions"); + await cmd.execute(["data loss|concurrent writer|race condition", "/memory"], makeCtx(fs) as never); + + expect(mockEmbed).toHaveBeenCalledWith("data loss|concurrent writer|race condition", "query"); + const opts = searchSpy.mock.calls[0][3] as { queryEmbedding: number[] | null }; + expect(opts.queryEmbedding).toEqual([0.4, 0.5, 0.6]); + searchSpy.mockRestore(); + }); + + it("still embeds an alternation with one other metachar", async () => { + mockEmbed.mockResolvedValueOnce([0.1]); + const client = makeClient([]); + const fs = await DeeplakeFs.create(client as never, "test", "/memory"); + const cmd = createGrepCommand(client as never, fs, "test"); + await cmd.execute(["deploy failed|rollback?", "/memory"], makeCtx(fs) as never); + expect(mockEmbed).toHaveBeenCalled(); + }); + + it("skips embedding on alternations with more than 8 alternatives", async () => { + mockEmbed.mockResolvedValue([0.5]); + const client = makeClient([]); + const fs = await DeeplakeFs.create(client as never, "test", "/memory"); + const cmd = createGrepCommand(client as never, fs, "test"); + await cmd.execute(["a1|a2|a3|a4|a5|a6|a7|a8|a9", "/memory"], makeCtx(fs) as never); + expect(mockEmbed).not.toHaveBeenCalled(); + }); + it("skips embedding on very short patterns (< 2 chars)", async () => { mockEmbed.mockClear(); const client = makeClient([]);