diff --git a/docs/analytics.html b/docs/analytics.html index a522f90..442f480 100644 --- a/docs/analytics.html +++ b/docs/analytics.html @@ -477,7 +477,15 @@

Top Queries

Tool Count Avg Results - Avg Score + + + Avg Cosine + diff --git a/src/__tests__/analytics-observability.test.ts b/src/__tests__/analytics-observability.test.ts index fccf130..bfdcbcf 100644 --- a/src/__tests__/analytics-observability.test.ts +++ b/src/__tests__/analytics-observability.test.ts @@ -9,6 +9,7 @@ import { ALL_TIME_DAYS, ROLLING_WINDOW_CAP_DAYS, BROWSE_QUERY_TEXT, + COSINE_SCORE_KIND, } from "../db/analytics.js"; import { generatePostSchemaMigration } from "../db/schema.js"; @@ -72,14 +73,21 @@ async function seed(db: PGlite, count: number, opts: SeedOpts = {}) { for (let i = 0; i < count; i++) { await db.query( `INSERT INTO query_log - (tool_name, query_text, result_count, top_score, latency_ms, + (tool_name, query_text, result_count, top_score, score_kind, latency_ms, source_name, session_id, request_source, created_at) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)`, + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10)`, [ "search-docs", opts.query_text ?? "q", opts.result_count ?? 5, opts.top_score === undefined ? 0.9 : opts.top_score, + // Mirror logQuery's write-boundary rule: a present score declares its + // scale, an absent one declares nothing. Score-based readers require + // the tag, so seeding without it would silently test the "unknown + // scale, excluded" path instead of the intended one. + (opts.top_score === undefined ? 0.9 : opts.top_score) == null + ? null + : COSINE_SCORE_KIND, 42, "docs", "sess-1", @@ -298,14 +306,17 @@ async function seedAt( ): Promise { await db.query( `INSERT INTO query_log - (tool_name, query_text, result_count, top_score, latency_ms, + (tool_name, query_text, result_count, top_score, score_kind, latency_ms, source_name, session_id, request_source, created_at) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)`, + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10)`, [ "search-docs", opts.query_text ?? "q", opts.result_count ?? 5, opts.top_score === undefined ? 0.9 : opts.top_score, + (opts.top_score === undefined ? 0.9 : opts.top_score) == null + ? null + : COSINE_SCORE_KIND, 42, "docs", "sess-1", diff --git a/src/__tests__/analytics.test.ts b/src/__tests__/analytics.test.ts index 167f8c9..b7f0b0c 100644 --- a/src/__tests__/analytics.test.ts +++ b/src/__tests__/analytics.test.ts @@ -18,6 +18,7 @@ import { REDACTED_QUERY_TEXT, P95_LATENCY_ROW_CAP, LOW_CONFIDENCE_SCORE_THRESHOLD, + COSINE_SCORE_KIND, normalizeRequestSource, DEFAULT_REQUEST_SOURCE, REQUEST_SOURCE_VALUES, @@ -63,6 +64,9 @@ describe("logQuery", () => { "how to install", 5, 0.92, + // score_kind: derived from top_score at the write boundary, so a present + // score is always tagged with the scale it lives on. + COSINE_SCORE_KIND, 42, "docs", "sess-123", @@ -93,6 +97,7 @@ describe("logQuery", () => { REDACTED_QUERY_TEXT, baseEntry.result_count, baseEntry.top_score, + COSINE_SCORE_KIND, baseEntry.latency_ms, baseEntry.source_name, baseEntry.session_id, @@ -119,8 +124,11 @@ describe("logQuery", () => { const [, params] = mockQuery.mock.calls[0]; expect(params[3]).toBeNull(); // top_score - expect(params[5]).toBeNull(); // source_name - expect(params[6]).toBeNull(); // session_id + // A NULL top_score must carry a NULL score_kind — an untagged absence, not + // a score on an unnamed scale. + expect(params[4]).toBeNull(); // score_kind + expect(params[6]).toBeNull(); // source_name + expect(params[7]).toBeNull(); // session_id }); it("persists the session_id passed on the entry (no longer hardcoded null)", async () => { @@ -131,7 +139,7 @@ describe("logQuery", () => { await logQuery({ ...baseEntry, session_id: "live-session-42" }); const [, params] = mockQuery.mock.calls[0]; - expect(params[6]).toBe("live-session-42"); + expect(params[7]).toBe("live-session-42"); }); it("coerces an unknown request_source to the default ('user')", async () => { @@ -141,7 +149,7 @@ describe("logQuery", () => { await logQuery({ ...baseEntry, request_source: "bogus-origin" }); const [, params] = mockQuery.mock.calls[0]; - expect(params[7]).toBe("user"); + expect(params[8]).toBe("user"); }); it("coerces an absent request_source to the default ('user')", async () => { @@ -151,7 +159,7 @@ describe("logQuery", () => { await logQuery(noSource); const [, params] = mockQuery.mock.calls[0]; - expect(params[7]).toBe("user"); + expect(params[8]).toBe("user"); }); it("persists a synthetic request_source verbatim", async () => { @@ -159,12 +167,12 @@ describe("logQuery", () => { await logQuery({ ...baseEntry, request_source: "synthetic" }); const [, params] = mockQuery.mock.calls[0]; - expect(params[7]).toBe("synthetic"); + expect(params[8]).toBe("synthetic"); }); // v1.15.2 attribution columns: client_ip, user_agent, blocked, block_reason. - // Positional indices on params are 8, 9, 10, 11 respectively (after the - // existing 8 fields tool_name..request_source). + // Positional indices on params are 9, 10, 11, 12 respectively (after the + // nine fields tool_name..request_source, which now include score_kind). it("persists client_ip and user_agent verbatim when provided", async () => { mockQuery.mockResolvedValueOnce({ rows: [] }); @@ -175,8 +183,8 @@ describe("logQuery", () => { }); const [, params] = mockQuery.mock.calls[0]; - expect(params[8]).toBe("203.0.113.10"); - expect(params[9]).toBe("Claude-User/1.0"); + expect(params[9]).toBe("203.0.113.10"); + expect(params[10]).toBe("Claude-User/1.0"); }); it("truncates a pathological user_agent to USER_AGENT_MAX_LEN chars", async () => { @@ -188,7 +196,7 @@ describe("logQuery", () => { await logQuery({ ...baseEntry, user_agent: huge }); const [, params] = mockQuery.mock.calls[0]; - expect((params[9] as string).length).toBe(256); + expect((params[10] as string).length).toBe(256); }); it("persists blocked=true with a block_reason", async () => { @@ -200,8 +208,8 @@ describe("logQuery", () => { }); const [, params] = mockQuery.mock.calls[0]; - expect(params[10]).toBe(true); - expect(params[11]).toBe("pattern:movie-box-office"); + expect(params[11]).toBe(true); + expect(params[12]).toBe("pattern:movie-box-office"); }); it("defaults blocked to false and block_reason to null when absent", async () => { @@ -211,8 +219,8 @@ describe("logQuery", () => { await logQuery(baseEntry); const [, params] = mockQuery.mock.calls[0]; - expect(params[10]).toBe(false); - expect(params[11]).toBeNull(); + expect(params[11]).toBe(false); + expect(params[12]).toBeNull(); }); }); @@ -1833,6 +1841,31 @@ describe("getAnalyticsSummary low-confidence metric", () => { expect(params).toContain(LOW_CONFIDENCE_SCORE_THRESHOLD); }); + it("gates the low-confidence FILTER on score_kind so a foreign scale can't be compared", async () => { + // The threshold lives on the cosine scale, so only rows that DECLARE that + // scale may be compared against it. Without this guard, legacy rows whose + // top_score holds an RRF rank score (ceiling ~0.033) compare below any + // cosine threshold and flag 100% of scored traffic — the bug this replaces. + mockSummaryQueries(); + await getAnalyticsSummary({}); + + const [sql, params] = mockQuery.mock.calls[1]; + expect(sql).toMatch(/score_kind = \$\d+/); + expect(params).toContain(COSINE_SCORE_KIND); + }); + + it("averages top_score only over cosine-scaled rows in getTopQueries", async () => { + // Same scale guard on the dashboard's Avg Cosine column: a legacy RRF row + // must not be folded into a mean presented as a cosine similarity. + mockQuery.mockResolvedValueOnce({ rows: [] }); + await getTopQueries(7, 50); + + const [sql, params] = mockQuery.mock.calls[0]; + expect(sql).toMatch(/avg\(top_score\) FILTER/); + expect(sql).toMatch(/score_kind = \$\d+/); + expect(params).toContain(COSINE_SCORE_KIND); + }); + it("threshold constant is 0.5 (matches the brief)", () => { expect(LOW_CONFIDENCE_SCORE_THRESHOLD).toBe(0.5); }); diff --git a/src/__tests__/relevance-score-scale.test.ts b/src/__tests__/relevance-score-scale.test.ts new file mode 100644 index 0000000..9024522 --- /dev/null +++ b/src/__tests__/relevance-score-scale.test.ts @@ -0,0 +1,438 @@ +import { + describe, + it, + expect, + vi, + beforeAll, + beforeEach, + afterAll, +} from "vitest"; +import { PGlite } from "@electric-sql/pglite"; +import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; +import type { SearchToolConfig, ChunkResult } from "../types.js"; + +// ----------------------------------------------------------------------------- +// Scale integrity of query_log.top_score. +// +// Retrieval produces two numbers per result on two different scales: +// `similarity` (a per-retriever RANKING score) and `cosine_similarity` (a 0-1 +// RELEVANCE score). Persisting the former broke every score-based analytic: +// in hybrid mode `similarity` is a Reciprocal Rank Fusion score whose ceiling +// is 2/(RRF_K+1) ≈ 0.0328, so a perfect match logged ~0.016 against a 0.5 +// low-confidence threshold and EVERY scored query was flagged. +// +// These tests pin the invariant that keeps the two scales from being confused +// again, at three levels: the reducer, the live tool -> logQuery path (driven +// through the REAL rrfMerge), and the SQL readers against real Postgres. +// ----------------------------------------------------------------------------- + +vi.mock("../db/analytics.js", async () => { + const actual = + await vi.importActual( + "../db/analytics.js", + ); + return { ...actual, logQuery: vi.fn().mockResolvedValue(undefined) }; +}); +vi.mock("../config.js", () => ({ + getServerConfig: vi.fn().mockReturnValue({}), + getAnalyticsConfig: vi.fn().mockReturnValue({ log_queries: true }), +})); + +import { rrfMerge, RRF_K } from "../db/queries.js"; +import { + topCosineScore, + COSINE_SCORE_KIND, + COSINE_SCORE_MAX, +} from "../relevance.js"; +import { + LOW_CONFIDENCE_SCORE_THRESHOLD, + logQuery, + getAnalyticsSummary, + getTopQueries, +} from "../db/analytics.js"; +import { generatePostSchemaMigration } from "../db/schema.js"; +import { __setPoolForTesting, __resetPoolForTesting } from "../db/client.js"; +import { registerSearchTool } from "../mcp/tools/search.js"; + +const mockLogQuery = vi.mocked(logQuery); + +/** Highest score the RRF ranking scale can ever produce: rank 1 in BOTH lists. */ +const MAX_RRF_SCORE = 2 / (RRF_K + 1); + +function makeChunk( + id: number, + cosine: number | null, + overrides: Partial = {}, +): ChunkResult { + return { + id, + source_name: "docs", + source_url: `https://docs.example.com/${id}`, + title: `Doc ${id}`, + content: `Content ${id}`, + repo_url: null, + file_path: `docs/${id}.md`, + start_line: null, + end_line: null, + language: null, + // A vector row's ranking score IS its cosine; a keyword row's is a ts_rank. + similarity: cosine ?? 0.04, + cosine_similarity: cosine, + ...overrides, + }; +} + +// ── The scale invariant itself ─────────────────────────────────────────────── + +describe("low-confidence threshold and the metric it is compared against", () => { + it("is derived from the cosine scale, not hard-coded onto it", () => { + expect(LOW_CONFIDENCE_SCORE_THRESHOLD).toBe(COSINE_SCORE_MAX * 0.5); + expect(LOW_CONFIDENCE_SCORE_THRESHOLD).toBeGreaterThan(0); + expect(LOW_CONFIDENCE_SCORE_THRESHOLD).toBeLessThanOrEqual( + COSINE_SCORE_MAX, + ); + }); + + it("sits far above the RRF ranking ceiling, so an RRF score can never be a valid input", () => { + // This is the arithmetic of the original bug stated as an assertion: the + // best possible RRF score (rank 1 in both retrievers) is still an order of + // magnitude below the threshold. Any metric bounded by MAX_RRF_SCORE would + // classify 100% of scored queries as low-confidence, so the threshold and + // a fusion score cannot legally share a column. + expect(MAX_RRF_SCORE).toBeLessThan(LOW_CONFIDENCE_SCORE_THRESHOLD / 10); + }); +}); + +// ── The reducer ────────────────────────────────────────────────────────────── + +describe("topCosineScore", () => { + it("returns the highest cosine, ignoring the ranking score", () => { + const results = [ + makeChunk(1, 0.42, { similarity: 0.0328 }), + makeChunk(2, 0.81, { similarity: 0.0164 }), + ]; + expect(topCosineScore(results)).toBeCloseTo(0.81); + }); + + it("returns null for an empty result set", () => { + expect(topCosineScore([])).toBeNull(); + }); + + it("returns null when every row is keyword-only (no comparable score)", () => { + expect(topCosineScore([makeChunk(1, null), makeChunk(2, null)])).toBeNull(); + }); + + it("skips keyword-only rows rather than treating them as zero", () => { + // A null must not be coerced to 0 — that would drag a genuinely good + // result set below the low-confidence threshold. + const best = topCosineScore([makeChunk(1, null), makeChunk(2, 0.77)]); + expect(best).toBeCloseTo(0.77); + }); + + it("ignores a non-finite cosine rather than propagating NaN", () => { + const results = [makeChunk(1, NaN), makeChunk(2, 0.6)]; + expect(topCosineScore(results)).toBeCloseTo(0.6); + }); +}); + +// ── rrfMerge carries the relevance score through the fusion ────────────────── + +describe("rrfMerge preserves cosine_similarity", () => { + it("overwrites similarity with the RRF score but keeps the cosine intact", () => { + const vector = [makeChunk(1, 0.88), makeChunk(2, 0.71)]; + const keyword = [makeChunk(3, null), makeChunk(1, null)]; + + const merged = rrfMerge(vector, keyword, 10); + const byId = new Map(merged.map((r) => [r.id, r])); + + // Ranking scale: every fused score is bounded by the RRF ceiling. + for (const r of merged) { + expect(r.similarity).toBeLessThanOrEqual(MAX_RRF_SCORE); + } + // Relevance scale: untouched by the fusion. + expect(byId.get(1)!.cosine_similarity).toBeCloseTo(0.88); + expect(byId.get(2)!.cosine_similarity).toBeCloseTo(0.71); + // A keyword-only hit has no comparable score and must stay null, not + // inherit the fused rank score. + expect(byId.get(3)!.cosine_similarity).toBeNull(); + }); + + it("keeps the vector row's cosine when a chunk appears in both lists", () => { + const merged = rrfMerge([makeChunk(7, 0.93)], [makeChunk(7, null)], 10); + expect(merged).toHaveLength(1); + expect(merged[0].cosine_similarity).toBeCloseTo(0.93); + expect(merged[0].similarity).toBeCloseTo(MAX_RRF_SCORE); + }); +}); + +// ── The live hybrid tool -> logQuery path ──────────────────────────────────── + +const hybridToolConfig: SearchToolConfig = { + name: "search-docs", + type: "search", + description: "Search the docs", + source: "docs", + default_limit: 5, + max_limit: 20, + result_format: "docs", + search_mode: "hybrid", +}; + +/** + * Drive the REAL registered search tool in hybrid mode over the REAL rrfMerge + * and return what it persisted to query_log. `hybridSearchChunks` is stubbed + * only to inject a deterministic candidate set — the fusion, the reduction to + * top_score, and the write boundary are all production code. + */ +async function runHybridSearch( + vector: ChunkResult[], + keyword: ChunkResult[], +): Promise<{ topScore: number | null; results: ChunkResult[] }> { + const fused = rrfMerge(vector, keyword, 5); + + const queries = await import("../db/queries.js"); + const spy = vi.spyOn(queries, "hybridSearchChunks").mockResolvedValue(fused); + + const server = new McpServer({ name: "t", version: "1.0.0" }); + registerSearchTool( + server, + { embed: vi.fn().mockResolvedValue([0.1, 0.2]), embedBatch: vi.fn() }, + hybridToolConfig, + ); + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + const client = new Client({ name: "c", version: "1.0.0" }); + await Promise.all([ + client.connect(clientTransport), + server.server.connect(serverTransport), + ]); + + await client.callTool({ + name: "search-docs", + arguments: { query: "useCopilotAction" }, + }); + await client.close(); + spy.mockRestore(); + + const entry = mockLogQuery.mock.calls.at(-1)![0]; + return { topScore: entry.top_score, results: fused }; +} + +describe("hybrid search logs a relevance score, not a rank score", () => { + beforeEach(() => { + mockLogQuery.mockClear(); + }); + + it("persists the best cosine even though the returned rows are RRF-ranked", async () => { + const { topScore, results } = await runHybridSearch( + [makeChunk(1, 0.87), makeChunk(2, 0.63)], + [makeChunk(3, null), makeChunk(1, null)], + ); + + // The rows the caller ranks by are still on the RRF scale... + expect(Math.max(...results.map((r) => r.similarity))).toBeLessThanOrEqual( + MAX_RRF_SCORE, + ); + // ...but what we PERSIST is the cosine. + expect(topScore).toBeCloseTo(0.87); + expect(topScore!).toBeGreaterThan(MAX_RRF_SCORE); + }); + + it("DISCRIMINATES between an excellent match and a poor one", async () => { + // The regression that matters. Under the old `Math.max(...similarity)` + // both of these log ~0.0164 — identical, and both below the threshold — + // so the metric carries no information. Reintroducing that scale mismatch + // fails this test on both the equality and the classification. + const excellent = await runHybridSearch( + [makeChunk(1, 0.87)], + [makeChunk(1, null)], + ); + const poor = await runHybridSearch( + [makeChunk(9, 0.21)], + [makeChunk(9, null)], + ); + + expect(excellent.topScore).not.toBeCloseTo(poor.topScore!); + expect(excellent.topScore!).toBeGreaterThan(LOW_CONFIDENCE_SCORE_THRESHOLD); + expect(poor.topScore!).toBeLessThan(LOW_CONFIDENCE_SCORE_THRESHOLD); + }); + + it("logs a null score when only keyword-only hits came back", async () => { + // No comparable score exists, so we record its absence rather than + // inventing one. The analytics layer reads NULL as "no score", never as a + // low score, so this does not manufacture a false content-gap signal. + const { topScore } = await runHybridSearch( + [], + [makeChunk(3, null), makeChunk(4, null)], + ); + expect(topScore).toBeNull(); + }); +}); + +// ── The SQL readers, against real Postgres ─────────────────────────────────── + +const QUERY_LOG_DDL_MARKER = + "-- Analytics: query_log table for tracking tool usage"; + +function extractQueryLogDdl(): string { + const full = generatePostSchemaMigration(); + const idx = full.indexOf(QUERY_LOG_DDL_MARKER); + if (idx < 0) { + throw new Error( + `Could not locate "${QUERY_LOG_DDL_MARKER}" in generatePostSchemaMigration(); ` + + `schema.ts may have been refactored — update the marker.`, + ); + } + return full.slice(idx); +} + +function poolFromPglite(db: PGlite) { + return { + query: (text: string, params?: unknown[]) => db.query(text, params), + connect: async () => ({ + query: (text: string, params?: unknown[]) => db.query(text, params), + release: () => {}, + }), + end: async () => db.close(), + }; +} + +describe("score-based readers fence off rows of unknown scale (PGlite)", () => { + let db: PGlite; + + beforeAll(async () => { + db = new PGlite(); + await db.waitReady; + await db.exec(extractQueryLogDdl()); + __setPoolForTesting(poolFromPglite(db)); + }); + + afterAll(async () => { + __resetPoolForTesting(); + await db.close(); + }); + + beforeEach(async () => { + await db.query("DELETE FROM query_log"); + }); + + /** A row as written BEFORE score_kind existed: an RRF value, no scale tag. */ + async function seedLegacyRrfRow(queryText: string): Promise { + await db.query( + `INSERT INTO query_log + (tool_name, query_text, result_count, top_score, score_kind, + latency_ms, source_name, session_id, request_source) + VALUES ($1, $2, 5, 0.0164, NULL, 40, 'docs', 'sess-1', 'user')`, + ["search-docs", queryText], + ); + } + + it("logQuery tags a present score with its scale and leaves an absent one untagged", async () => { + const actual = + await vi.importActual( + "../db/analytics.js", + ); + await actual.logQuery({ + tool_name: "search-docs", + query_text: "scored", + result_count: 3, + top_score: 0.72, + latency_ms: 10, + source_name: "docs", + session_id: null, + }); + await actual.logQuery({ + tool_name: "search-docs", + query_text: "unscored", + result_count: 3, + top_score: null, + latency_ms: 10, + source_name: "docs", + session_id: null, + }); + + const { rows } = await db.query<{ + query_text: string; + score_kind: string | null; + }>("SELECT query_text, score_kind FROM query_log ORDER BY id"); + expect(rows).toEqual([ + { query_text: "scored", score_kind: COSINE_SCORE_KIND }, + { query_text: "unscored", score_kind: null }, + ]); + }); + + it("does NOT count a legacy RRF-scaled row as low confidence", async () => { + // 0.0164 < 0.5 numerically, but it is not a cosine. Counting it would + // reproduce the 100%-false-positive signal this change removes. + await seedLegacyRrfRow("legacy hybrid query"); + + const summary = await getAnalyticsSummary({}, 7); + expect(summary.total_queries_window).toBe(1); + expect(summary.low_confidence_count_window).toBe(0); + expect(summary.low_confidence_rate_window).toBe(0); + }); + + it("counts a genuinely weak cosine row, and spares a strong one", async () => { + const actual = + await vi.importActual( + "../db/analytics.js", + ); + await actual.logQuery({ + tool_name: "search-docs", + query_text: "weak match", + result_count: 4, + top_score: 0.21, + latency_ms: 10, + source_name: "docs", + session_id: null, + }); + await actual.logQuery({ + tool_name: "search-docs", + query_text: "strong match", + result_count: 4, + top_score: 0.87, + latency_ms: 10, + source_name: "docs", + session_id: null, + }); + + const summary = await getAnalyticsSummary({}, 7); + expect(summary.total_queries_window).toBe(2); + expect(summary.low_confidence_count_window).toBe(1); + expect(summary.low_confidence_rate_window).toBeCloseTo(0.5); + }); + + it("excludes legacy rows from the dashboard's Avg Cosine column", async () => { + // Mixing an RRF 0.0164 into the mean would render a number that is neither + // a cosine nor a rank score. The column reads "—" instead. + await seedLegacyRrfRow("legacy hybrid query"); + const [legacy] = await getTopQueries(7, 10); + expect(legacy.query_text).toBe("legacy hybrid query"); + expect(legacy.count).toBe(1); + expect(legacy.avg_top_score).toBeNull(); + }); + + it("averages only the cosine-scaled rows when both kinds share a query", async () => { + const actual = + await vi.importActual( + "../db/analytics.js", + ); + await seedLegacyRrfRow("mixed history"); + await actual.logQuery({ + tool_name: "search-docs", + query_text: "mixed history", + result_count: 5, + top_score: 0.8, + latency_ms: 10, + source_name: "docs", + session_id: null, + }); + + const [row] = await getTopQueries(7, 10); + expect(row.count).toBe(2); + // 0.8 alone — NOT (0.8 + 0.0164) / 2. + expect(row.avg_top_score!).toBeCloseTo(0.8); + }); +}); diff --git a/src/__tests__/search-analytics.test.ts b/src/__tests__/search-analytics.test.ts index 4a4ba8d..4bbf71f 100644 --- a/src/__tests__/search-analytics.test.ts +++ b/src/__tests__/search-analytics.test.ts @@ -49,6 +49,10 @@ function makeChunkResult(overrides: Partial = {}): ChunkResult { end_line: null, language: null, similarity: 0.9, + // Mirror what searchChunks actually returns: a vector hit's ranking score + // and its cosine similarity are the same number. Overrides can decouple + // them to model a hybrid (RRF-ranked) row. + cosine_similarity: overrides.similarity ?? 0.9, ...overrides, }; } diff --git a/src/__tests__/search-hybrid.test.ts b/src/__tests__/search-hybrid.test.ts index ce92f10..04631b7 100644 --- a/src/__tests__/search-hybrid.test.ts +++ b/src/__tests__/search-hybrid.test.ts @@ -50,6 +50,11 @@ function makeChunkResult(overrides: Partial = {}): ChunkResult { end_line: null, language: null, similarity: 0.9, + // Mirror what searchChunks actually returns: a vector hit's ranking score + // and its cosine similarity are the same number. Hybrid cases override + // `cosine_similarity` explicitly to model an RRF-ranked row, where + // `similarity` is a fusion score and the cosine is the real relevance. + cosine_similarity: overrides.similarity ?? 0.9, ...overrides, }; } diff --git a/src/db/analytics.ts b/src/db/analytics.ts index 726f47e..7af32b8 100644 --- a/src/db/analytics.ts +++ b/src/db/analytics.ts @@ -1,4 +1,5 @@ import { getPool } from "./client.js"; +import { COSINE_SCORE_KIND, COSINE_SCORE_MAX } from "../relevance.js"; // --------------------------------------------------------------------------- // Constants @@ -59,12 +60,24 @@ export const P95_LATENCY_ROW_CAP = 100000; * that look like hits but aren't actually relevant. Exported so tool handlers, * readers, and tests share a single source of truth. * - * Predicate (matches the brief): `result_count > 0 AND top_score < 0.5`. - * `top_score IS NULL` (e.g. browse/keyword rows that never compute a cosine - * score) is intentionally NOT low-confidence — absence of a score is not a - * low score. + * DERIVED from {@link COSINE_SCORE_MAX} (the midpoint of the scale) rather than + * hard-coded, so it cannot drift onto a different scale than the metric it is + * compared against. That drift is exactly what broke this metric before: the + * threshold sat at 0.5 while hybrid mode persisted an RRF rank score whose + * ceiling is ~0.033, so every scored hybrid query was flagged low-confidence. + * + * Predicate: `result_count > 0 AND score_kind = 'cosine' AND top_score < + * threshold`. `top_score IS NULL` (browse/keyword rows that never compute a + * cosine) is intentionally NOT low-confidence — absence of a score is not a + * low score — and neither is a row whose scale is unknown (score_kind NULL). */ -export const LOW_CONFIDENCE_SCORE_THRESHOLD = 0.5; +export const LOW_CONFIDENCE_SCORE_THRESHOLD = COSINE_SCORE_MAX * 0.5; + +// Re-exported so the scale contract reads as one surface: every consumer of +// the low-confidence threshold also needs the kind tag and the scale bound, +// and they are defined in src/relevance.ts (which owns the retrieval-side +// reducer) to keep this module free of a dependency on ChunkResult. +export { COSINE_SCORE_KIND, COSINE_SCORE_MAX }; /** * Canonical request-origin tags persisted on `query_log.request_source`. @@ -125,7 +138,22 @@ export interface QueryLogEntry { tool_name: string; query_text: string; result_count: number; + /** + * Best RELEVANCE score across the returned results, as a cosine similarity + * in [0, 1], or null when no returned result carried one (an empty result + * set, a browse call, or a keyword-only match). Never a ranking score: a + * ts_rank or an RRF fusion score belongs to a different scale and must be + * logged as null, not squeezed into this column. See + * {@link COSINE_SCORE_KIND}. + */ top_score: number | null; + /** + * Scale declaration for {@link top_score} — {@link COSINE_SCORE_KIND} when a + * score is present, null when it is not. Optional on the entry so existing + * call sites compile; the writer derives it from `top_score` so the two can + * never disagree. + */ + score_kind?: string | null; latency_ms: number; source_name: string | null; session_id: string | null; @@ -242,6 +270,14 @@ export interface TopQuery { tool_name: string; count: number; avg_result_count: number | null; + /** + * Mean best-match COSINE similarity (0-1) across this query's events — + * the dashboard's "Avg Cosine" column. Averaged only over rows whose + * `score_kind` declares the cosine scale, so a legacy row holding an RRF + * rank score can never drag the average onto a different scale. Null when + * no event in the window carried a cosine (keyword-only or browse traffic, + * or history predating `score_kind`). + */ avg_top_score: number | null; } @@ -366,15 +402,25 @@ export async function logQuery( const blocked = entry.blocked ?? false; const blockReason = entry.block_reason ?? null; const clientIp = entry.client_ip ?? null; + // Derive score_kind from top_score at the write boundary rather than trusting + // the caller, so the column and the value it describes can never disagree: a + // present score is always tagged 'cosine' (tools only ever log a cosine — see + // topCosineScore in src/relevance.ts), and an absent score is always + // untagged. A caller-supplied kind is honored only when a score is present, + // which leaves room for a future second scale without letting a NULL score + // carry a kind. + const scoreKind = + entry.top_score == null ? null : (entry.score_kind ?? COSINE_SCORE_KIND); try { await pool.query( - `INSERT INTO query_log (tool_name, query_text, result_count, top_score, latency_ms, source_name, session_id, request_source, client_ip, user_agent, blocked, block_reason) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12)`, + `INSERT INTO query_log (tool_name, query_text, result_count, top_score, score_kind, latency_ms, source_name, session_id, request_source, client_ip, user_agent, blocked, block_reason) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13)`, [ entry.tool_name, text, entry.result_count, entry.top_score, + scoreKind, entry.latency_ms, entry.source_name, entry.session_id, @@ -772,6 +818,7 @@ export async function getAnalyticsSummary( const rs2 = buildRequestSourceClause(filter, dw2.nextIdx); const redactedIdx2 = rs2.nextIdx; const lowConfIdx2 = redactedIdx2 + 1; + const scoreKindIdx2 = lowConfIdx2 + 1; const summaryBase = [ ...dw2.clauses, ...rs2.clauses, @@ -785,6 +832,12 @@ export async function getAnalyticsSummary( // threshold is bound, not inlined, so LOW_CONFIDENCE_SCORE_THRESHOLD stays // the single source of truth. `top_score IS NOT NULL` is part of the FILTER // so NULL-score rows (browse/keyword) never count as low confidence. + // + // `score_kind = 'cosine'` is the SCALE GUARD: the threshold lives on the + // cosine scale, so only rows that declare that scale may be compared against + // it. It excludes legacy rows written before score_kind existed, whose + // top_score may hold an RRF rank score (ceiling ~0.033) that would compare + // below ANY cosine threshold and flag 100% of scored queries. const summaryRes = await pool.query( `SELECT count(*)::int AS total, @@ -792,6 +845,7 @@ export async function getAnalyticsSummary( count(*) FILTER ( WHERE result_count > 0 AND top_score IS NOT NULL + AND score_kind = $${scoreKindIdx2} AND top_score < $${lowConfIdx2} )::int AS low_confidence, COALESCE(avg(latency_ms)::int, 0) AS avg_latency, @@ -805,6 +859,7 @@ export async function getAnalyticsSummary( ...rs2.params, REDACTED_QUERY_TEXT, LOW_CONFIDENCE_SCORE_THRESHOLD, + COSINE_SCORE_KIND, ], ); @@ -1054,14 +1109,24 @@ export async function getTopQueries( tool_name, count(*)::int AS count, avg(result_count) FILTER (WHERE result_count >= 0)::real AS avg_result_count, - avg(top_score) FILTER (WHERE top_score IS NOT NULL)::real AS avg_top_score + avg(top_score) FILTER ( + WHERE top_score IS NOT NULL + AND score_kind = $${redactedIdx + 1} + )::real AS avg_top_score FROM query_log ${where} GROUP BY query_text, tool_name HAVING bool_or(result_count > 0) ORDER BY count DESC - LIMIT $${redactedIdx + 1}`, - [...fp, ...dw.params, ...rs.params, REDACTED_QUERY_TEXT, limit], + LIMIT $${redactedIdx + 2}`, + [ + ...fp, + ...dw.params, + ...rs.params, + REDACTED_QUERY_TEXT, + COSINE_SCORE_KIND, + limit, + ], ); return rows.map((r: Record) => { diff --git a/src/db/queries.ts b/src/db/queries.ts index 21417c4..55fa25c 100644 --- a/src/db/queries.ts +++ b/src/db/queries.ts @@ -340,6 +340,11 @@ export async function searchChunks( // Coerce to a finite number: a non-numeric similarity would Number() to // NaN and corrupt the similarity sort order / top_score downstream. similarity: toFiniteNumber(r.similarity), + // Vector rows carry a real cosine similarity, so ranking score and + // relevance score coincide here. Recorded separately anyway: rrfMerge + // overwrites `similarity` with the fused rank score, and this is the copy + // analytics reads (see ChunkResult.cosine_similarity). + cosine_similarity: toFiniteNumber(r.similarity), })); } @@ -445,6 +450,9 @@ export async function textSearchChunks( // Coerce to a finite number: a non-numeric similarity would Number() to // NaN and corrupt the similarity sort order / top_score downstream. similarity: toFiniteNumber(r.similarity), + // ts_rank is not on the cosine scale and there is no embedding distance + // for a keyword-only hit, so this row contributes no relevance score. + cosine_similarity: null, })); } @@ -500,6 +508,13 @@ export async function hybridSearchChunks( * where k = 60 (standard constant from the original RRF paper). * Documents appearing in only one list get a single-term score. * + * The fused score is written to `similarity`, so on the returned rows + * `similarity` is a RANK score bounded by 2/(RRF_K+1) ≈ 0.033 — not a cosine + * similarity. `cosine_similarity` is carried through untouched from the + * canonical (vector-preferred) result, so callers that need a relevance score + * on the 0-1 cosine scale read THAT field. Persisting `similarity` as a + * relevance metric is a scale error; see src/mcp/tools/search.ts. + * * Exported for direct unit testing of the merge logic. */ export const RRF_K = 60; @@ -544,7 +559,9 @@ export function rrfMerge( .sort((a, b) => b.rrfScore - a.rrfScore) .slice(0, limit); - // Return ChunkResult[] with similarity set to the RRF score + // Return ChunkResult[] with similarity set to the RRF score. The spread + // preserves `cosine_similarity` from the canonical result — that is the only + // relevance signal that survives the fusion, so do not drop or overwrite it. return sorted.map(({ rrfScore, result }) => ({ ...result, similarity: rrfScore, diff --git a/src/db/schema.ts b/src/db/schema.ts index 4dc0f8a..a7d15c7 100644 --- a/src/db/schema.ts +++ b/src/db/schema.ts @@ -106,6 +106,7 @@ CREATE TABLE IF NOT EXISTS query_log ( query_text TEXT NOT NULL, result_count INTEGER NOT NULL, top_score REAL, + score_kind TEXT, latency_ms INTEGER NOT NULL, source_name TEXT, session_id TEXT, @@ -140,6 +141,22 @@ ALTER TABLE query_log ADD COLUMN IF NOT EXISTS block_reason TEXT; CREATE INDEX IF NOT EXISTS idx_query_log_blocked ON query_log (blocked); CREATE INDEX IF NOT EXISTS idx_query_log_client_ip ON query_log (client_ip); +-- score_kind declares the SCALE of top_score. Added after query_log shipped, so +-- ADD COLUMN IF NOT EXISTS keeps the migration idempotent; the CREATE TABLE +-- above carries it for fresh installs. +-- +-- There is deliberately NO BACKFILL. Historical rows were written under the old +-- mode-dependent semantics: vector mode logged a cosine, keyword mode logged a +-- ts_rank, and hybrid mode logged a Reciprocal Rank Fusion score capped at +-- ~0.033. Those scales are not recoverable per-row after the fact (an RRF 0.016 +-- carries no derivable cosine), and guessing from the value would silently +-- reinterpret history. Leaving score_kind NULL marks such a row "unknown +-- scale"; every score-based reader (the low-confidence FILTER in +-- getAnalyticsSummary and avg_top_score in getTopQueries) now requires +-- score_kind = 'cosine', so legacy rows are EXCLUDED rather than misread. The +-- score-based cards therefore start empty and refill as new traffic lands. +ALTER TABLE query_log ADD COLUMN IF NOT EXISTS score_kind TEXT; + -- Webhook delivery tracking CREATE TABLE IF NOT EXISTS webhook_deliveries ( id SERIAL PRIMARY KEY, diff --git a/src/mcp/tools/knowledge.ts b/src/mcp/tools/knowledge.ts index 6cec191..1f3af18 100644 --- a/src/mcp/tools/knowledge.ts +++ b/src/mcp/tools/knowledge.ts @@ -11,6 +11,7 @@ import { getFaqChunksByIds, searchChunks, } from "../../db/queries.js"; +import { topCosineScore } from "../../relevance.js"; import { logQuery } from "../../db/analytics.js"; import { getAnalyticsConfig } from "../../config.js"; import { checkBlocklist } from "../abuse-blocklist.js"; @@ -255,6 +256,11 @@ export function registerKnowledgeTool( qualifying.push({ ...faqChunk, similarity: result.similarity, + // Carry the cosine through with the ranking score. getFaqChunks* + // select no similarity column of their own, so without this the + // merged row would look score-less and drop out of the + // low-confidence / Avg Cosine analytics. + cosine_similarity: result.cosine_similarity, }); } } @@ -262,10 +268,12 @@ export function registerKnowledgeTool( // Fire-and-forget analytics logging const analyticsConfig = getAnalyticsConfig(); - const topScore = - mergedResults.length > 0 - ? Math.max(...mergedResults.map((r) => r.similarity)) - : null; + // Same contract as the search tool: log the best COSINE similarity, + // so query_log.top_score is one metric on one scale across every + // tool. This path is vector-only, so the cosine and the ranking + // score coincide — reducing over cosine_similarity keeps it that way + // if a future change fuses in another retriever. See topCosineScore. + const topScore = topCosineScore(mergedResults); logQuery( { tool_name: toolConfig.name, diff --git a/src/mcp/tools/search.ts b/src/mcp/tools/search.ts index feffcde..bd41524 100644 --- a/src/mcp/tools/search.ts +++ b/src/mcp/tools/search.ts @@ -7,6 +7,7 @@ import { textSearchChunks, hybridSearchChunks, } from "../../db/queries.js"; +import { topCosineScore } from "../../relevance.js"; import { logQuery } from "../../db/analytics.js"; import { getAnalyticsConfig } from "../../config.js"; import { checkBlocklist } from "../abuse-blocklist.js"; @@ -105,7 +106,12 @@ export function registerSearchTool( .max(1) .optional() .describe( - "Minimum similarity score (0-1). Results below this threshold are filtered out.", + "Minimum cosine similarity (0-1) for semantically matched results. " + + "In vector mode it filters the returned results. In hybrid mode it " + + "raises the semantic floor of the vector half BEFORE the results are " + + "fused with keyword matches, so a keyword-only match can still be " + + "returned below this score. Ignored in keyword mode, which has no " + + "comparable score.", ), version: z .string() @@ -230,10 +236,14 @@ export function registerSearchTool( // Fire-and-forget analytics logging (always captures, regardless of analytics.enabled) const logQueries = getAnalyticsConfig()?.log_queries ?? true; const latencyMs = Date.now() - startMs; - const topScore = - results.length > 0 - ? Math.max(...results.map((r) => r.similarity)) - : null; + // Persist the best COSINE similarity, never `similarity`. In hybrid + // mode `similarity` has been overwritten with the RRF fusion score + // (ceiling ≈ 0.033) and in keyword mode it is a ts_rank — neither is + // comparable to the 0-1 cosine scale the low-confidence threshold and + // the dashboard's Avg Cosine column are defined on. Keyword mode + // therefore logs NULL here, which analytics reads as "no score", not + // "a low score". See topCosineScore. + const topScore = topCosineScore(results); logQuery( { tool_name: toolConfig.name, diff --git a/src/relevance.ts b/src/relevance.ts new file mode 100644 index 0000000..b12d7ce --- /dev/null +++ b/src/relevance.ts @@ -0,0 +1,54 @@ +// Relevance scoring contract shared by the retrieval tools and the analytics +// layer. +// +// Retrieval produces TWO distinct numbers per result and they must not be +// confused. `ChunkResult.similarity` is a RANKING score whose scale depends on +// the retriever (cosine in vector mode, ts_rank in keyword mode, a Reciprocal +// Rank Fusion score in hybrid mode). `ChunkResult.cosine_similarity` is a +// RELEVANCE score, always on the 0-1 cosine scale or null. Only the latter may +// be persisted, aggregated, or compared against a threshold. + +import type { ChunkResult } from "./types.js"; + +/** + * Value written to `query_log.score_kind` when `top_score` holds a cosine + * similarity — the only kind a tool ever writes today. The column exists to + * fence HISTORY: rows predating it hold mode-dependent values under a NULL + * score_kind, so score-based readers require this value and skip the rest + * rather than reinterpreting them. See src/db/schema.ts for why there is no + * backfill. + */ +export const COSINE_SCORE_KIND = "cosine"; + +/** + * Upper bound of the metric `query_log.top_score` is recorded on — cosine + * similarity, which pgvector reports as `1 - (embedding <=> query)` in [0, 1]. + * Exported so any threshold compared against `top_score` can be derived from + * the scale instead of hard-coded onto it. + */ +export const COSINE_SCORE_MAX = 1; + +/** + * Best RELEVANCE score across a result set: the highest cosine similarity any + * row carries, or null when none does (an empty set, or a set made entirely of + * keyword-only hits). + * + * This is what belongs in `query_log.top_score` — NOT `Math.max(...similarity)`. + * Maxing `similarity` persists a cosine in vector mode, a ts_rank in keyword + * mode, and an RRF fusion score capped at 2/(RRF_K+1) ≈ 0.033 in hybrid mode. + * Comparing those against a single 0-1 threshold is a scale error: it flagged + * every scored hybrid query as low-confidence and rendered a meaningless "Avg + * Score" on the dashboard. Reducing over `cosine_similarity` keeps one metric + * on one scale in all three modes, at the cost of returning null when only + * keyword hits came back — which the analytics layer already treats as "no + * score", not "a low score". + */ +export function topCosineScore(results: ChunkResult[]): number | null { + let best: number | null = null; + for (const r of results) { + const cosine = r.cosine_similarity; + if (typeof cosine !== "number" || !Number.isFinite(cosine)) continue; + if (best === null || cosine > best) best = cosine; + } + return best; +} diff --git a/src/types.ts b/src/types.ts index 818e1ac..94c690c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -566,7 +566,30 @@ export interface ChunkResult { start_line: number | null; end_line: number | null; language: string | null; + /** + * RANKING score. Its scale depends on which retriever produced the row: + * cosine similarity (0-1) from `searchChunks`, ts_rank from + * `textSearchChunks`, and a fused Reciprocal Rank Fusion score + * (max 2/(RRF_K+1) ≈ 0.033) from `rrfMerge`. It orders results and nothing + * more — it is NOT comparable across modes and must NEVER be persisted as a + * relevance metric. Use {@link cosine_similarity} for that. + */ similarity: number; + /** + * RELEVANCE score: the true cosine similarity (0-1) of this chunk against + * the query embedding, or null when the row has no comparable semantic score + * (a keyword-only hit — ts_rank is not on the cosine scale). + * + * Kept separate from {@link similarity} because `rrfMerge` OVERWRITES + * `similarity` with the fused rank score, destroying the cosine value. This + * field survives the merge, so analytics (`query_log.top_score`, the + * dashboard's Avg Cosine column, the low-confidence flag) reads one metric + * on one scale regardless of `search_mode`. + * + * Optional so non-retrieval producers of ChunkResult-shaped rows (Atlas + * dedup, bash related-files) compile unchanged; absent reads as "no cosine". + */ + cosine_similarity?: number | null; } export interface FaqChunkResult extends ChunkResult {