From 06c26eb8c8457900c3ffa3eb53b0f6c992cdd57e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 23:01:29 -0400 Subject: [PATCH] fix: call the retrieval RPCs by their deployed names (GIT-93) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit recall has two paths: a local vector index, and a Supabase RPC fallback used whenever that index is not ready. The fallback returned PGRST202 on every call. scarSearch and semanticSearch built the RPC name by taking the TABLE prefix and appending a verb: const rpcName = `${getTableName("").replace(/_$/, "")}_scar_search`; which is "orchestra_scar_search" under GITMEM_TABLE_PREFIX=orchestra_ and "gitmem_scar_search" by default. Enumerating the functions PostgREST actually exposes turned up neither, under any prefix — nor any *_scar_search at all. The deployed names are match_ and match_
_weighted, so the RPC is named after the table it searches, not after the prefix with a verb glued on. The docstrings claiming gitmem_scar_search / gitmem_semantic_search described a function that does not exist; they are corrected too. So this was never a prefix mismatch that a non-default deployment tripped over. The fallback had not worked on any deployment since it was written. It stayed invisible because it is only reached while the local index is cold — a window that happens to include the first recall of every session, the one the SessionStart hook triggers, which is the recall that gates the work about to be done. _weighted is the correct one of the pair for scarSearch: that function is documented as scar search with severity weighting, and it is the variant returning decay_multiplier, which recall consumes. It takes match_threshold rather than similarity_threshold, so the body changes with the name. project_filter is deliberately not sent. This stands in for the unified cross-project vector cache; filtering here would make the cold path return a narrower set than the warm path it substitutes for — a silent behaviour change on precisely the calls that are hardest to notice. Verified end to end against the real store: a recall issued before "[startup] GitMem initialized" now returns 3 scars with no PGRST202, where it previously returned none. +5 tests (1180 -> 1185), hermetic (fetch stubbed, asserting the URL), and confirmed to fail against the pre-fix names. Not fixed here: with nothing surfaced, confirm_scars still reports "No recall-surfaced scars to confirm. Proceed freely." — a green result for a failed retrieval. Tracked on GIT-93 as step 2, and it is the reason this bug could persist unnoticed. Co-Authored-By: Claude Opus 5 --- src/services/supabase-client.ts | 34 +++-- .../services/scar-search-rpc-name.test.ts | 118 ++++++++++++++++++ 2 files changed, 145 insertions(+), 7 deletions(-) create mode 100644 tests/unit/services/scar-search-rpc-name.test.ts diff --git a/src/services/supabase-client.ts b/src/services/supabase-client.ts index 43b5172..171629c 100644 --- a/src/services/supabase-client.ts +++ b/src/services/supabase-client.ts @@ -190,7 +190,7 @@ export async function upsertRecord( /** * Semantic search across tables * - * Generates an embedding for the query, then calls the gitmem_semantic_search + * Generates an embedding for the query, then calls the match_
* RPC function directly via PostgREST. */ export async function semanticSearch( @@ -211,8 +211,11 @@ export async function semanticSearch( return []; } - // Call the RPC function directly via PostgREST - const rpcName = `${getTableName("").replace(/_$/, "")}_semantic_search`; + // GIT-93: same defect as scarSearch below — this built `${prefix}_semantic_search`, + // which exists under no prefix. The deployed function is match_
, and it + // takes similarity_threshold (unlike the _weighted variant), which is what the + // body already sends. + const rpcName = `match_${getTableName("learnings")}`; const url = `${SUPABASE_URL}/rest/v1/rpc/${rpcName}`; const response = await fetch(url, { @@ -579,7 +582,7 @@ export async function loadScarsWithEmbeddings( /** * Scar search with severity weighting * - * Generates an embedding for the query, then calls the gitmem_scar_search + * Generates an embedding for the query, then calls the match_
_weighted * RPC function directly via PostgREST. No Edge Function required. */ export async function scarSearch( @@ -600,8 +603,20 @@ export async function scarSearch( return []; } - // Call the RPC function directly via PostgREST - const rpcName = `${getTableName("").replace(/_$/, "")}_scar_search`; + // GIT-93: the RPC is named after the TABLE it searches, not after the prefix + // with a verb appended. This built `${prefix}_scar_search` — "orchestra_scar_search" + // under GITMEM_TABLE_PREFIX=orchestra_, and "gitmem_scar_search" by default. + // Neither exists: PostgREST exposes match_orchestra_learnings_weighted, and a + // survey of the deployed functions found no *_scar_search under any prefix. So + // this fallback returned PGRST202 on every call, on every deployment, since it + // was written — invisible because it is only reached while the local vector + // index is cold. + // + // _weighted is the right one of the pair: this function is documented as scar + // search WITH SEVERITY WEIGHTING, and it is the variant that returns + // decay_multiplier, which recall consumes. Note it takes match_threshold, not + // similarity_threshold — the unweighted variant takes the latter. + const rpcName = `match_${getTableName("learnings")}_weighted`; const url = `${SUPABASE_URL}/rest/v1/rpc/${rpcName}`; const response = await fetch(url, { @@ -614,7 +629,12 @@ export async function scarSearch( body: JSON.stringify({ query_embedding: `[${embedding.join(",")}]`, match_count: matchCount, - similarity_threshold: 0.0, + match_threshold: 0.0, + // project_filter is deliberately not sent. The primary path this falls back + // from is the unified CROSS-PROJECT vector cache, so filtering here would + // make the fallback return a different, narrower result set than the path + // it stands in for — a silent behaviour change on exactly the cold-start + // calls that are hardest to notice. }), signal: AbortSignal.timeout(15_000), }); diff --git a/tests/unit/services/scar-search-rpc-name.test.ts b/tests/unit/services/scar-search-rpc-name.test.ts new file mode 100644 index 0000000..2754cfd --- /dev/null +++ b/tests/unit/services/scar-search-rpc-name.test.ts @@ -0,0 +1,118 @@ +/** + * GIT-93: the retrieval RPCs must be called by the name that is actually + * deployed, which is derived from the TABLE being searched. + * + * scarSearch and semanticSearch built their RPC name by taking the table prefix + * and appending a verb: `${prefix}_scar_search` / `${prefix}_semantic_search`. + * That produced "orchestra_scar_search" under GITMEM_TABLE_PREFIX=orchestra_ and + * "gitmem_scar_search" by default. A survey of the functions PostgREST exposes + * found neither, under any prefix — the deployed names are match_
and + * match_
_weighted. Every call returned PGRST202. + * + * It went unnoticed because these are fallbacks: recall only reaches them while + * the local vector index is still loading. In that window — which includes the + * first recall of every session, the one the SessionStart hook triggers — + * retrieval returned nothing at all. + * + * These tests assert the URL rather than the response, because the defect was + * entirely in name construction. They are hermetic: fetch is stubbed, so they + * fail on a wrong name rather than on network conditions, and they hold for a + * deployment whose functions this developer cannot reach. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +const ORIGINAL_ENV = { ...process.env }; + +/** Captures the URL and body of the single fetch each search performs. */ +function stubFetch(): { calls: Array<{ url: string; body: Record }> } { + const calls: Array<{ url: string; body: Record }> = []; + vi.stubGlobal("fetch", vi.fn(async (url: string, init?: { body?: string }) => { + calls.push({ url: String(url), body: init?.body ? JSON.parse(init.body) : {} }); + return { + ok: true, + status: 200, + json: async () => [], + text: async () => "[]", + } as unknown as Response; + })); + return { calls }; +} + +async function loadClient(prefix: string) { + process.env.GITMEM_TABLE_PREFIX = prefix; + process.env.SUPABASE_URL = "https://example.supabase.co"; + process.env.SUPABASE_SERVICE_ROLE_KEY = "test-key"; + process.env.GITMEM_TIER = "pro"; + // Reset the module registry so SUPABASE_URL and the prefix are re-read. + vi.resetModules(); + vi.doMock("../../../src/services/embedding.js", () => ({ + embed: async () => new Array(1536).fill(0.01), + })); + return import("../../../src/services/supabase-client.js"); +} + +describe("GIT-93: retrieval RPC names are derived from the table, not the prefix", () => { + beforeEach(() => { vi.resetModules(); }); + + afterEach(() => { + vi.unstubAllGlobals(); + vi.doUnmock("../../../src/services/embedding.js"); + process.env = { ...ORIGINAL_ENV }; + }); + + it("scarSearch calls match_
_weighted under a non-default prefix", async () => { + const { calls } = stubFetch(); + const client = await loadClient("orchestra_"); + + await client.scarSearch("any query", 3); + + expect(calls).toHaveLength(1); + expect(calls[0].url).toContain("/rest/v1/rpc/match_orchestra_learnings_weighted"); + // The name that was being built before the fix. Asserted explicitly so this + // test fails loudly if the prefix-plus-verb construction ever returns. + expect(calls[0].url).not.toContain("orchestra_scar_search"); + }); + + it("scarSearch sends match_threshold, which is what the weighted function takes", async () => { + const { calls } = stubFetch(); + const client = await loadClient("orchestra_"); + + await client.scarSearch("any query", 3); + + // The unweighted variant takes similarity_threshold; sending the wrong one + // to the weighted function silently loses the threshold. + expect(calls[0].body).toHaveProperty("match_threshold"); + expect(calls[0].body).not.toHaveProperty("similarity_threshold"); + }); + + it("scarSearch does not narrow the fallback to one project", async () => { + const { calls } = stubFetch(); + const client = await loadClient("orchestra_"); + + await client.scarSearch("any query", 3); + + // It stands in for the unified CROSS-PROJECT vector cache. Filtering here + // would make the cold path return a narrower set than the warm path. + expect(calls[0].body).not.toHaveProperty("project_filter"); + }); + + it("semanticSearch calls match_
", async () => { + const { calls } = stubFetch(); + const client = await loadClient("orchestra_"); + + await client.semanticSearch({ query: "any query", match_count: 5 }); + + expect(calls[0].url).toContain("/rest/v1/rpc/match_orchestra_learnings"); + expect(calls[0].url).not.toContain("orchestra_semantic_search"); + }); + + it("tracks the prefix rather than hardcoding one deployment's table", async () => { + const { calls } = stubFetch(); + const client = await loadClient("gitmem_"); + + await client.scarSearch("any query", 3); + + expect(calls[0].url).toContain("/rest/v1/rpc/match_gitmem_learnings_weighted"); + }); +});