From b9257710424517f37785d6a5c53222d990e69e63 Mon Sep 17 00:00:00 2001 From: "mads.thines" Date: Mon, 21 Sep 2026 18:30:57 +0000 Subject: [PATCH 1/9] feat(relevant): grade lexical relevance with ts_rank_cd and cut the candidate window by rank MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `GET /memories/relevant` scored relevance as a BINARY 1-or-nothing because `ts_rank_cd` is not projectable through PostgREST's query grammar, and for the same reason cut its candidate window in `updated_at desc` order — so the best-matching lesson in a store with more than CANDIDATE_LIMIT matches could fall outside the window and never be scored at all. Migration 00112 moves the candidate fetch into `lorekit_memory_relevant_candidates`, which returns `ts_rank_cd` alongside each row and orders the window by it whenever a query is present. The handler maps the raw, unbounded rank into the [0,1) factor the scorer consumes with `normalizeLexicalRank` — strictly increasing, so it can never reorder what Postgres ranked. Ranking stays deterministic and embedding-free: a pure Postgres function over the existing `fts` column, no model, no vector index, no external call. The composite scorer does NOT move into SQL. Postgres selects and grades; `lesson-rank.ts`/`lessons-pure.mjs` still order, because the ranking is set-relative and has to agree with the CLI hook's ordering — an agreement only `lesson-rank-parity.spec.ts` can hold two TypeScript-family implementations to. --- CLAUDE.md | 1 + docs/decisions.md | 58 +++ .../src/auth/tenant-scope-usage.spec.ts | 49 ++- packages/mcp-core/src/ranking/lesson-rank.ts | 53 +++ .../mcp-core/src/ranking/lexical-rank.spec.ts | 137 +++++++ packages/schemas/src/llms/template.md | 17 +- packages/schemas/src/shared/relevant.ts | 36 +- packages/web/public/llms.txt | 17 +- .../functions/_shared/ranking/lesson-rank.ts | 53 +++ .../functions/_shared/schemas/relevant.ts | 36 +- supabase/functions/memories/CLAUDE.md | 42 ++- .../functions/memories/handlers/relevant.ts | 179 +++++---- .../migrations/00112_relevant_ranking.sql | 197 ++++++++++ supabase/tests/migrations.test.sql | 344 ++++++++++++++++++ 14 files changed, 1101 insertions(+), 118 deletions(-) create mode 100644 packages/mcp-core/src/ranking/lexical-rank.spec.ts create mode 100644 supabase/migrations/00112_relevant_ranking.sql diff --git a/CLAUDE.md b/CLAUDE.md index c3f3259a9..765de1f4f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -446,3 +446,4 @@ their rationale inline. **Do not relitigate these.** - **An omitted scope on a read means EVERYWHERE, not `global`** — [rationale](./docs/decisions.md#an-omitted-scope-means-everywhere-not-global) - **A tool's `inputSchema` carries NO top-level `oneOf`/`anyOf`/`allOf`** — [rationale](./docs/decisions.md#a-tools-inputschema-carries-no-top-level-oneofanyofallof) - **A skill's `metadata.version` must be bumped on any content change, and CI enforces it** — [rationale](./docs/decisions.md#a-skills-metadataversion-must-be-bumped-on-any-content-change-and-ci-enforces-it) +- **Graded relevance is lexical `ts_rank_cd`, never a vector score** — [rationale](./docs/decisions.md#graded-relevance-is-lexical-ts_rank_cd-never-a-vector-score) diff --git a/docs/decisions.md b/docs/decisions.md index 4a08b6f10..90e4e35a8 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -775,3 +775,61 @@ is the whole point of 00105. The **Read** stat card stays scope-level: `usage_ev scope but nothing about the age or counters of the memories it returned, so a per-lesson threshold is unanswerable there. §108 pins the narrowing, the self-exclusion interaction, and that all-null params equal omitting them. + +## Graded relevance is lexical `ts_rank_cd`, never a vector score + +`GET /memories/relevant` ranks on four factors, and the relevance one used to be BINARY: a row +that matched the FTS predicate scored `1`, a row that did not was never returned, and nothing +scored between. That was not an oversight — `ts_rank`/`ts_rank_cd` is not projectable through +PostgREST's query grammar, so from the PostgREST side there was no graded value to be had. The +same limitation cut the candidate window in `updated_at desc` order rather than by rank, so on a +store with more matching rows than `CANDIDATE_LIMIT` the BEST-matching lesson could fall outside +the window and never be scored at all. + +Migration 00112 moves the candidate fetch into `lorekit_memory_relevant_candidates`, which +returns `ts_rank_cd(fts, websearch_to_tsquery('english', q))` alongside the row and — when a +query is present — orders the window by it. Two things follow: ordering among matches now +reflects *how well* each row matched, and the rows the cap discards are the ones that matched +worst rather than the ones that happened to be older. + +**The decision worth not relitigating is what relevance IS.** It is a lexical score: term +overlap and proximity, computed by a pure Postgres function over the same `fts` column +`POST /memories/search` already matches on. It is NOT an embedding similarity, and the obvious +"upgrade" — fusing a vector score into the same factor — is out of scope by choice, not by +backlog order. Retrieval here is deterministic and embedding-free: identical inputs produce +identical output, run to run, with no model inference on read or on write, no vector index to +keep warm, and no external call on the request path. A ranking a user cannot reproduce is a +ranking they cannot debug, and for a store whose whole job is telling an agent which of its own +past lessons to re-read, reproducibility is worth more than semantic recall. The +`memories.embedding` column and its dormant opt-in pipeline +([embeddings.md](./embeddings.md)) stay exactly that — dormant, and off this path. + +`ts_rank_cd` rather than `ts_rank` because cover density rewards query terms that appear close +together, which is the difference between a lesson actually about "migration backfill" and one +that mentions migrations in its first paragraph and backfills in its last. + +**The normalization is a named constant, not an inline expression.** `ts_rank_cd` is unbounded +above and the scorer needs `[0,1]`, so `normalizeLexicalRank` maps `r → r / (r + s)`: strictly +increasing (it can never reorder what Postgres ranked), `0` at `0`, asymptotic to `1`. +`s = LEXICAL_RANK_SATURATION = 0.1` is anchored to a schema fact rather than taste — +`memories.fts` applies no `setweight`, so every lexeme carries Postgres's default D weight of +`0.1`, and one occurrence of one query lexeme is therefore a `ts_rank_cd` of `0.1`. Saturating +there makes "matched about as well as a single term does" read as the midpoint of the factor. +Saturating at `1` instead would squash the whole plausible range into the bottom half of the +scale, so a graded signal would arrive with most of its say already spent. Every `score` the +endpoint reports depends on this constant and nothing else in the repo would notice it moving, +so it is pinned by value in `lexical-rank.spec.ts` and its anchor is pinned in +`migrations.test.sql` §112 AC-4. + +**What this changes for callers.** `min_score` now gates across the whole range. Under binary +relevance no matched score could drop below `(1 + 0.5) / 4 = 0.375` — relevance at `1`, outcome +never below its `0.5` cold-start prior — so any `min_score` at or under `0.375` was a no-op. +Grading removes that floor, which is the point, but it means a threshold chosen against the old +behaviour is now stricter than it was. The `factors` block still reports the four raw values so +a caller can see exactly which one moved. + +**The scorer did NOT move into SQL, and must not.** Postgres selects and grades; the composite +score stays in `lesson-rank.ts` / `lessons-pure.mjs`. The ranking is set-relative (salience +normalises against the most-recurring candidate) and it has to agree exactly with the CLI hook's +ordering — an agreement `lesson-rank-parity.spec.ts` can hold two TypeScript-family +implementations to and could not hold a plpgsql third copy to at all. diff --git a/packages/mcp-core/src/auth/tenant-scope-usage.spec.ts b/packages/mcp-core/src/auth/tenant-scope-usage.spec.ts index cb7187f4d..4b0c419d4 100644 --- a/packages/mcp-core/src/auth/tenant-scope-usage.spec.ts +++ b/packages/mcp-core/src/auth/tenant-scope-usage.spec.ts @@ -124,8 +124,15 @@ describe('tenant-scope usage guard (edge read handlers)', () => { * kept its tests, and quietly stopped being covered by the predicate that made * it safe. Removing a name from this list is only ever correct alongside adding * it to another one. + * + * `relevant` left for the SAME reason, one migration later: 00112 moved its + * candidate fetch into `lorekit_memory_relevant_candidates` so Postgres could + * grade it with `ts_rank_cd`, which took the query out of + * `applyRestTenantScope`'s reach exactly as 00067 did for `list`. It is pinned + * by `RPC_BACKED_ROW_READS` below — AND by the tenancy guard after it, because + * unlike `list` it is a read whose visibility WIDENS to org rows. */ -const REST_READ_HANDLERS = ['get', 'relevant', 'search'] as const; +const REST_READ_HANDLERS = ['get', 'search'] as const; const readHandlerSource = (name: string) => readFileSync(path.resolve(here, `../../../../supabase/functions/memories/handlers/${name}.ts`), 'utf8'); @@ -222,7 +229,7 @@ const RPC_BACKED_SCOPE_READS = ['scopes', 'tags', 'activity', 'read-activity', ' * same pin — but the two are not one list, because a future aggregate and a * future row read are added for different reasons. */ -const RPC_BACKED_ROW_READS = ['list', 'utility'] as const; +const RPC_BACKED_ROW_READS = ['list', 'utility', 'relevant'] as const; /** * The allowlist expression, spelled either inline at the call site or bound @@ -298,6 +305,44 @@ describe('key-scope usage guard (RPC-backed row reads)', () => { }); }); +/** + * Drift guard: an RPC-backed read whose visibility WIDENS to org rows must pass + * the key's TENANCY, not only its scope allowlist. + * + * `p_key_scopes` is the half the guards above cover. It is not the whole + * boundary: a key can also be narrowed to `personal` or to a chosen set of orgs + * (00068), and in SQL that half is `lorekit_api_token_org_allowed`, which the + * RPCs default to `'all'` / `'{}'` — the UNRESTRICTED values. A handler that + * sends `p_key_scopes` and forgets the other two compiles, runs, passes every + * guard above, and hands a `personal` key every org row its OWNER can see. + * + * Why this is a separate list from `RPC_BACKED_ROW_READS`: not every RPC-backed + * read widens. `utility` is account-wide by construction, and the aggregates + * emit counts rather than rows. What earns a name here is reading rows whose + * visibility comes from ORG MEMBERSHIP rather than ownership — `list` and + * `relevant` — because that is the visibility the key's tenancy exists to + * narrow back down. + */ +const ORG_WIDENING_RPC_READS = ['list', 'relevant'] as const; + +describe('key-tenancy usage guard (org-widening RPC reads)', () => { + it.each(ORG_WIDENING_RPC_READS)('%s passes the key tenancy to its RPC', (name) => { + const src = readHandlerSource(name); + // The expressions, not merely the parameter names: `p_key_org_access: 'all'` + // would satisfy a name-only check while being precisely the fail-open value + // this guard exists to catch. + expect(src).toMatch(/p_key_org_access:\s*keyRestriction\(auth\)\?\.orgAccess\s*\?\?\s*'all'/); + expect(src).toMatch(/p_key_org_ids:\s*keyRestriction\(auth\)\?\.orgIds\s*\?\?\s*\[\]/); + }); + + it.each(ORG_WIDENING_RPC_READS)('%s sends the tenancy alongside the scope allowlist', (name) => { + // All three travel together or the boundary has a hole; a handler carrying + // one and not the others is the shape this pins against. + const src = readHandlerSource(name); + expect(passesKeyScopes(src)).toBe(true); + }); +}); + describe('key-scope usage guard (RPC-backed per-scope reads)', () => { it.each(RPC_BACKED_SCOPE_READS)('%s passes p_key_scopes to its RPC', (name) => { // The key's allowlist, not merely a mention of it in a comment: matched as diff --git a/packages/mcp-core/src/ranking/lesson-rank.ts b/packages/mcp-core/src/ranking/lesson-rank.ts index 222702eab..9c976acd8 100644 --- a/packages/mcp-core/src/ranking/lesson-rank.ts +++ b/packages/mcp-core/src/ranking/lesson-rank.ts @@ -155,6 +155,59 @@ export function normalizeRelevance(value: unknown): number { return Math.min(1, Math.max(0, n)); } +/** + * The `ts_rank_cd` value at which the lexical relevance factor reads 0.5. + * + * NOT an arbitrary tuning knob — it is anchored to a fact about the schema. + * `memories.fts` is `to_tsvector('english', key || ' ' || value)` with no + * `setweight` call anywhere (migration 00001), so every lexeme in it carries + * Postgres's default D weight, which is `0.1`. A cover containing exactly one + * query lexeme therefore contributes `0.1` to `ts_rank_cd`, and that is the + * weakest thing a real match can be. Putting the saturation point THERE makes + * "matched about as well as a single term does" read as the midpoint of the + * factor, with denser and multi-term covers ranging above it. + * + * The obvious alternative — saturating at `1` — is monotone and bounded too, + * but it maps the entire plausible `ts_rank_cd` range (roughly 0.1 to 1) into + * `[0.09, 0.5]`, so a graded signal would arrive squashed into the bottom half + * of its own scale and lose most of its say in the weighted average. Both + * choices are deterministic; this one spends the range on values that occur. + * + * Changing it changes every `score` this endpoint reports, so it is pinned by + * `lexical-rank.spec.ts` rather than left to drift. + */ +export const LEXICAL_RANK_SATURATION = 0.1; + +/** + * Map a raw Postgres `ts_rank_cd` value onto the `[0,1)` relevance factor + * `scoreLesson` consumes. + * + * `ts_rank_cd` is UNBOUNDED above — it grows with how many query terms a cover + * holds and how tightly they sit — so it cannot be handed to the scorer as-is + * without letting one dense match swamp every other factor. `r / (r + s)` is + * the saturating map: strictly increasing in `r` (so it never reorders what + * Postgres ranked), `0` at `0`, asymptotic to `1`, and with no discontinuity to + * land on. + * + * Absent, negative or unreadable input is `0` — the same answer + * `normalizeRelevance` gives, and for the same reason: no evidence of a lexical + * match is not the same claim as a weak one. A no-query request therefore keeps + * scoring exactly as it did before relevance was graded. + * + * The RANGE IS `[0,1)`, never `1`: no finite `ts_rank_cd` saturates it. That is + * deliberate — reaching 1 would claim a perfect match exists, and cover density + * has no such maximum to certify. + */ +export function normalizeLexicalRank( + value: unknown, + saturation: number = LEXICAL_RANK_SATURATION, +): number { + const n = typeof value === 'string' ? Number(value) : value; + if (typeof n !== 'number' || !Number.isFinite(n) || n <= 0) return 0; + const s = Number.isFinite(saturation) && saturation > 0 ? saturation : LEXICAL_RANK_SATURATION; + return n / (n + s); +} + /** * Normalize an outcome value into [0,1]. Absent or unreadable input returns * `COLD_START_OUTCOME_PRIOR` — the deliberate asymmetry vs `normalizeRelevance` diff --git a/packages/mcp-core/src/ranking/lexical-rank.spec.ts b/packages/mcp-core/src/ranking/lexical-rank.spec.ts new file mode 100644 index 000000000..823f19d00 --- /dev/null +++ b/packages/mcp-core/src/ranking/lexical-rank.spec.ts @@ -0,0 +1,137 @@ +import { describe, it, expect } from 'vitest'; +import { + normalizeLexicalRank, + normalizeRelevance, + scoreLesson, + rankLessons, + LEXICAL_RANK_SATURATION, +} from './lesson-rank.js'; + +/** + * PINS THE ts_rank_cd → relevance MAPPING. + * + * `GET /memories/relevant` asks Postgres for `ts_rank_cd` (migration 00112) and + * hands the raw, UNBOUNDED value to `normalizeLexicalRank` before the scorer + * ever sees it. Every `score` the endpoint reports therefore depends on this + * one function, and nothing else in the repo would notice if it changed: the + * response would still validate, the parity spec would still pass (relevance is + * supplied explicitly on both sides of it), and the only symptom would be that + * the endpoint quietly started recommending different lessons. + * + * So the mapping is pinned by value, not merely by property. A deliberate + * change to it has to come here and rewrite the numbers, which is exactly the + * visibility the change deserves. + */ +describe('normalizeLexicalRank: the pinned mapping', () => { + it('saturates at the schema-anchored default D weight', () => { + // `memories.fts` applies no `setweight`, so every lexeme carries Postgres's + // default D weight of 0.1 (migration 00001). Changing this constant is a + // change to every score the endpoint reports. + expect(LEXICAL_RANK_SATURATION).toBe(0.1); + }); + + it('maps a single-term cover onto the midpoint of the factor', () => { + // r = 0.1 is one query lexeme in one cover — the weakest a real match can + // be — and it reads 0.5, not 0.09. + expect(normalizeLexicalRank(0.1)).toBeCloseTo(0.5, 12); + }); + + it('maps the rest of the plausible ts_rank_cd range by value', () => { + expect(normalizeLexicalRank(0.2)).toBeCloseTo(2 / 3, 12); + expect(normalizeLexicalRank(0.3)).toBeCloseTo(0.75, 12); + expect(normalizeLexicalRank(0.5)).toBeCloseTo(5 / 6, 12); + expect(normalizeLexicalRank(1)).toBeCloseTo(10 / 11, 12); + expect(normalizeLexicalRank(10)).toBeCloseTo(100 / 101, 12); + }); + + it('is strictly increasing, so it can never reorder what Postgres ranked', () => { + // The load-bearing property: the RPC cuts its candidate window by + // `ts_rank_cd` desc, and this map must preserve that order or the window and + // the scoring would disagree about which rows matched best. + const ranks = [0.01, 0.05, 0.1, 0.11, 0.2, 0.37, 1, 2.5, 40]; + const mapped = ranks.map((r) => normalizeLexicalRank(r)); + for (let i = 1; i < mapped.length; i++) { + expect(mapped[i]).toBeGreaterThan(mapped[i - 1]); + } + }); + + it('stays inside [0,1) — asymptotic, never 1', () => { + for (const r of [0.0001, 0.1, 1, 1e3, 1e9]) { + const v = normalizeLexicalRank(r); + expect(v).toBeGreaterThan(0); + expect(v).toBeLessThan(1); + } + }); + + it('scores absent, zero, negative and unreadable input at 0', () => { + // The same answer `normalizeRelevance` gives, and for the same reason: no + // evidence of a lexical match is not a weak match. A no-`q` request arrives + // here as `null` on every row and must keep scoring exactly as it did + // before relevance was graded. + for (const bad of [null, undefined, 0, -1, -0.5, NaN, Infinity, -Infinity, '', 'lots', {}, []]) { + expect(normalizeLexicalRank(bad)).toBe(0); + } + expect(normalizeLexicalRank(null)).toBe(normalizeRelevance(null)); + }); + + it('reads a numeric string, because a driver may hand back a numeric as text', () => { + expect(normalizeLexicalRank('0.1')).toBeCloseTo(0.5, 12); + }); + + it('falls back to the pinned saturation on an unusable override', () => { + for (const bad of [0, -1, NaN, Infinity]) { + expect(normalizeLexicalRank(0.1, bad as number)).toBeCloseTo(0.5, 12); + } + // A usable override is honoured — the knob exists so a test can vary it. + expect(normalizeLexicalRank(1, 1)).toBeCloseTo(0.5, 12); + }); + + it('is deterministic — the same input returns the same bits, every time', () => { + const once = normalizeLexicalRank(0.37); + for (let i = 0; i < 100; i++) expect(normalizeLexicalRank(0.37)).toBe(once); + }); +}); + +describe('graded relevance changes the ordering the endpoint produces', () => { + const NOW = Date.parse('2026-09-01T00:00:00.000Z'); + const updatedAt = new Date(NOW - 86400000).toISOString(); + + /** Two rows identical in every factor but how well they matched. */ + const twoTermVsOne = [ + // `ts_rank_cd` for a cover holding two query lexemes vs one. + { scope: 'repo::a/b', key: 'one-term', value: 'x', seen_count: 3, updated_at: updatedAt, relevance: normalizeLexicalRank(0.1) }, + { scope: 'repo::a/b', key: 'two-terms', value: 'y', seen_count: 3, updated_at: updatedAt, relevance: normalizeLexicalRank(0.2) }, + ]; + + it('a row matching two query terms outranks one matching a single term', () => { + const ranked = rankLessons(twoTermVsOne, { now: NOW }); + expect(ranked.map((r) => r.entry.key)).toEqual(['two-terms', 'one-term']); + }); + + it('the binary form could not tell them apart — this is the regression', () => { + // Before 00112 every matched row arrived with `relevance: 1`, so the two + // rows above were identical in all four factors and only the key tiebreak + // separated them. That is the ordering this PR exists to replace. + const binary = twoTermVsOne.map((r) => ({ ...r, relevance: 1 })); + const ranked = rankLessons(binary, { now: NOW }); + expect(ranked[0].score).toBe(ranked[1].score); + expect(ranked.map((r) => r.entry.key)).toEqual(['one-term', 'two-terms']); + }); + + it('a weak match can now score below the old binary floor of 0.375', () => { + // The `min_score` contract changes with grading. Under binary relevance a + // matched row scored at least `(1 + 0.5) / 4 = 0.375` — the relevance term + // at 1 and the outcome term at its cold-start prior — so any `min_score` + // below that was a no-op. A faintly-matching, stale, unrecurring row now + // falls under it, which is the whole point of grading. + const weakAndStale = { + scope: 'global', + key: 'faint', + value: 'z', + seen_count: 1, + updated_at: new Date(NOW - 365 * 86400000).toISOString(), + relevance: normalizeLexicalRank(0.005), + }; + expect(scoreLesson(weakAndStale, { now: NOW, maxSeenCount: 1 })).toBeLessThan(0.375); + }); +}); diff --git a/packages/schemas/src/llms/template.md b/packages/schemas/src/llms/template.md index 746f18b45..bde66cd50 100644 --- a/packages/schemas/src/llms/template.md +++ b/packages/schemas/src/llms/template.md @@ -296,9 +296,10 @@ that difference is the whole game — the newest writes are usually one task's i `GET /memories/relevant` is the ranked shortlist. It scores each candidate on **recency** (exponential decay, 14-day half-life), **salience** (`log1p(seen_count)`, normalised across the -candidates), **relevance** (full-text match on `q`) and **outcome** (applied/resolution history — -`1` on an outcome bus, `0.75` carried to a PR, `0.5` cold-start prior when unproven), and returns a -compact index: +candidates), **relevance** (graded lexical match on `q` — Postgres `ts_rank_cd`, so a lesson whose +terms sit close together outranks one that merely mentions them) and **outcome** +(applied/resolution history — `1` on an outcome bus, `0.75` carried to a PR, `0.5` cold-start prior +when unproven), and returns a compact index: ```bash curl -H "Authorization: Bearer lk_ro_…" \ @@ -311,7 +312,7 @@ q=migration+backfill&scopes=repo::acme/app,global&limit=5" "entries": [ { "scope": "repo::acme/app", "key": "migration-order", "hook": "Always add the column before the backfill runs.", - "score": 0.74, "factors": { "recency": 0.61, "salience": 0.85, "relevance": 1, "outcome": 0.5 }, + "score": 0.69, "factors": { "recency": 0.61, "salience": 0.85, "relevance": 0.8, "outcome": 0.5 }, "seen_count": 9, "updated_at": "2026-07-30T09:12:00.000Z" } ], "candidates": 47 @@ -320,14 +321,18 @@ q=migration+backfill&scopes=repo::acme/app,global&limit=5" | Param | Default | Meaning | |-------|---------|---------| -| `q` | — | Free-text query. **Optional** — omit it and the ranking is recency + salience, i.e. "what matters generally". | +| `q` | — | Free-text query. **Optional** — omit it and the ranking is recency + salience, i.e. "what matters generally". With `q` the candidate window is cut by lexical rank, so the best matches are the ones that survive it; without `q` it is cut by recency. | | `scopes` | all visible | Comma-separated, **most-specific first**; the order breaks ties, so a project lesson wins over the global one it ties with. | | `limit` | `10` | 1–50. | -| `min_score` | `0` | Drop weak hits when injecting automatically — an irrelevant lesson every turn is worse than none. Note: with `q` set, matched hits floor at `(1 + 0.5) / 4 = 0.375` (relevance is binary and outcome never sinks below its `0.5` prior today), so a value ≤ `0.375` is a no-op; finer gating arrives with graded relevance. | +| `min_score` | `0` | Drop weak hits when injecting automatically — an irrelevant lesson every turn is worse than none. It now gates across the whole range: relevance used to be binary, which floored every matched hit at `(1 + 0.5) / 4 = 0.375` and made any lower value a no-op. Graded relevance removed that floor, so a `min_score` tuned against the old behaviour is stricter than it was — re-tune it against the `score` values you actually see. | Bodies are not returned — fetch the ones you want with `memory.read`. `candidates` is how many matched before ranking, so you can tell a shortlist from the whole set. +Ranking here is **deterministic and embedding-free**: the same request over the same rows returns +the same order, every time. `relevance` is a lexical score — term overlap and proximity computed by +Postgres — never a semantic or vector similarity, and nothing on this path calls a model. + ## Limits | Limit | Default | diff --git a/packages/schemas/src/shared/relevant.ts b/packages/schemas/src/shared/relevant.ts index 284153314..ae8517b0d 100644 --- a/packages/schemas/src/shared/relevant.ts +++ b/packages/schemas/src/shared/relevant.ts @@ -34,6 +34,10 @@ export const RelevantQuerySchema = z.object({ * Free-text query. Matched with Postgres `websearch` FTS over `key || value`, * the same index `POST /memories/search` uses — so a term that finds a lesson * there finds it here, and only the ORDER differs. + * + * With `q` set the candidate window is cut by lexical RANK (`ts_rank_cd`), + * not by recency, so the best matches are the ones that survive it. Without + * `q` there is no rank to cut by and the window stays `updated_at desc`. */ q: z.string().max(1000).optional(), /** @@ -55,15 +59,21 @@ export const RelevantQuerySchema = z.object({ * something weak" — which is what a per-turn hook needs, since injecting an * irrelevant lesson on every prompt is worse than injecting nothing. * - * Mind the floor this reaches today. When `q` is set every matched candidate - * carries `relevance: 1` — relevance is binary here (see the handler's - * docblock) — and the outcome factor never sinks below its cold-start prior - * of `0.5` (it maps to `1`, `0.75`, or the prior — never `0`). So with the - * endpoint's four equal weights no matched score drops below - * `(1 + 0.5) / 4 = 0.375`, and a `min_score` at or under `0.375` is therefore - * a no-op. Above `0.375` it still discriminates, on recency and salience. A - * `min_score` that gates across the whole range needs graded FTS relevance, - * which lands with the ranked-relevance RPC (PR 11). + * IT NOW GATES ACROSS THE WHOLE RANGE, and that is a behaviour change worth + * knowing about if you set it before migration 00112. Relevance used to be + * BINARY — every matched candidate carried `relevance: 1` — so with the + * endpoint's four equal weights and an outcome factor that never sinks below + * its `0.5` cold-start prior, no matched score could drop under + * `(1 + 0.5) / 4 = 0.375`, and any `min_score` at or below that was a no-op. + * + * Relevance is now the graded lexical rank (`ts_rank_cd`, normalised into + * `[0,1)`), so a faint match scores a faint relevance and the old floor is + * gone: a weak, stale, unrecurring hit can now land below `0.375` and be + * filtered. That is the point of grading — but it means a `min_score` chosen + * against the binary behaviour is now stricter than it was, and a value that + * used to pass everything may start dropping real hits. Re-tune it against + * the `score` values you actually see rather than carrying the old number + * over. */ min_score: z.coerce.number().min(0).max(1).optional().default(0), }); @@ -81,6 +91,14 @@ export const RelevantEntrySchema = z.object({ factors: z.object({ recency: z.number(), salience: z.number(), + /** + * Graded lexical relevance in `[0,1)` — Postgres `ts_rank_cd` over the + * `fts` column, saturated into the range (migration 00112). `0` means the + * request carried no `q` at all, never "matched, but badly": a row that did + * not match is not returned. It is a LEXICAL score — term overlap and + * proximity, deterministically computed — and never a semantic or + * embedding similarity. + */ relevance: z.number(), outcome: z.number(), }), diff --git a/packages/web/public/llms.txt b/packages/web/public/llms.txt index 7e4e7fa9f..03bff6b28 100644 --- a/packages/web/public/llms.txt +++ b/packages/web/public/llms.txt @@ -779,9 +779,10 @@ that difference is the whole game — the newest writes are usually one task's i `GET /memories/relevant` is the ranked shortlist. It scores each candidate on **recency** (exponential decay, 14-day half-life), **salience** (`log1p(seen_count)`, normalised across the -candidates), **relevance** (full-text match on `q`) and **outcome** (applied/resolution history — -`1` on an outcome bus, `0.75` carried to a PR, `0.5` cold-start prior when unproven), and returns a -compact index: +candidates), **relevance** (graded lexical match on `q` — Postgres `ts_rank_cd`, so a lesson whose +terms sit close together outranks one that merely mentions them) and **outcome** +(applied/resolution history — `1` on an outcome bus, `0.75` carried to a PR, `0.5` cold-start prior +when unproven), and returns a compact index: ```bash curl -H "Authorization: Bearer lk_ro_…" \ @@ -794,7 +795,7 @@ q=migration+backfill&scopes=repo::acme/app,global&limit=5" "entries": [ { "scope": "repo::acme/app", "key": "migration-order", "hook": "Always add the column before the backfill runs.", - "score": 0.74, "factors": { "recency": 0.61, "salience": 0.85, "relevance": 1, "outcome": 0.5 }, + "score": 0.69, "factors": { "recency": 0.61, "salience": 0.85, "relevance": 0.8, "outcome": 0.5 }, "seen_count": 9, "updated_at": "2026-07-30T09:12:00.000Z" } ], "candidates": 47 @@ -803,14 +804,18 @@ q=migration+backfill&scopes=repo::acme/app,global&limit=5" | Param | Default | Meaning | |-------|---------|---------| -| `q` | — | Free-text query. **Optional** — omit it and the ranking is recency + salience, i.e. "what matters generally". | +| `q` | — | Free-text query. **Optional** — omit it and the ranking is recency + salience, i.e. "what matters generally". With `q` the candidate window is cut by lexical rank, so the best matches are the ones that survive it; without `q` it is cut by recency. | | `scopes` | all visible | Comma-separated, **most-specific first**; the order breaks ties, so a project lesson wins over the global one it ties with. | | `limit` | `10` | 1–50. | -| `min_score` | `0` | Drop weak hits when injecting automatically — an irrelevant lesson every turn is worse than none. Note: with `q` set, matched hits floor at `(1 + 0.5) / 4 = 0.375` (relevance is binary and outcome never sinks below its `0.5` prior today), so a value ≤ `0.375` is a no-op; finer gating arrives with graded relevance. | +| `min_score` | `0` | Drop weak hits when injecting automatically — an irrelevant lesson every turn is worse than none. It now gates across the whole range: relevance used to be binary, which floored every matched hit at `(1 + 0.5) / 4 = 0.375` and made any lower value a no-op. Graded relevance removed that floor, so a `min_score` tuned against the old behaviour is stricter than it was — re-tune it against the `score` values you actually see. | Bodies are not returned — fetch the ones you want with `memory.read`. `candidates` is how many matched before ranking, so you can tell a shortlist from the whole set. +Ranking here is **deterministic and embedding-free**: the same request over the same rows returns +the same order, every time. `relevance` is a lexical score — term overlap and proximity computed by +Postgres — never a semantic or vector similarity, and nothing on this path calls a model. + ## Limits | Limit | Default | diff --git a/supabase/functions/_shared/ranking/lesson-rank.ts b/supabase/functions/_shared/ranking/lesson-rank.ts index 222702eab..9c976acd8 100644 --- a/supabase/functions/_shared/ranking/lesson-rank.ts +++ b/supabase/functions/_shared/ranking/lesson-rank.ts @@ -155,6 +155,59 @@ export function normalizeRelevance(value: unknown): number { return Math.min(1, Math.max(0, n)); } +/** + * The `ts_rank_cd` value at which the lexical relevance factor reads 0.5. + * + * NOT an arbitrary tuning knob — it is anchored to a fact about the schema. + * `memories.fts` is `to_tsvector('english', key || ' ' || value)` with no + * `setweight` call anywhere (migration 00001), so every lexeme in it carries + * Postgres's default D weight, which is `0.1`. A cover containing exactly one + * query lexeme therefore contributes `0.1` to `ts_rank_cd`, and that is the + * weakest thing a real match can be. Putting the saturation point THERE makes + * "matched about as well as a single term does" read as the midpoint of the + * factor, with denser and multi-term covers ranging above it. + * + * The obvious alternative — saturating at `1` — is monotone and bounded too, + * but it maps the entire plausible `ts_rank_cd` range (roughly 0.1 to 1) into + * `[0.09, 0.5]`, so a graded signal would arrive squashed into the bottom half + * of its own scale and lose most of its say in the weighted average. Both + * choices are deterministic; this one spends the range on values that occur. + * + * Changing it changes every `score` this endpoint reports, so it is pinned by + * `lexical-rank.spec.ts` rather than left to drift. + */ +export const LEXICAL_RANK_SATURATION = 0.1; + +/** + * Map a raw Postgres `ts_rank_cd` value onto the `[0,1)` relevance factor + * `scoreLesson` consumes. + * + * `ts_rank_cd` is UNBOUNDED above — it grows with how many query terms a cover + * holds and how tightly they sit — so it cannot be handed to the scorer as-is + * without letting one dense match swamp every other factor. `r / (r + s)` is + * the saturating map: strictly increasing in `r` (so it never reorders what + * Postgres ranked), `0` at `0`, asymptotic to `1`, and with no discontinuity to + * land on. + * + * Absent, negative or unreadable input is `0` — the same answer + * `normalizeRelevance` gives, and for the same reason: no evidence of a lexical + * match is not the same claim as a weak one. A no-query request therefore keeps + * scoring exactly as it did before relevance was graded. + * + * The RANGE IS `[0,1)`, never `1`: no finite `ts_rank_cd` saturates it. That is + * deliberate — reaching 1 would claim a perfect match exists, and cover density + * has no such maximum to certify. + */ +export function normalizeLexicalRank( + value: unknown, + saturation: number = LEXICAL_RANK_SATURATION, +): number { + const n = typeof value === 'string' ? Number(value) : value; + if (typeof n !== 'number' || !Number.isFinite(n) || n <= 0) return 0; + const s = Number.isFinite(saturation) && saturation > 0 ? saturation : LEXICAL_RANK_SATURATION; + return n / (n + s); +} + /** * Normalize an outcome value into [0,1]. Absent or unreadable input returns * `COLD_START_OUTCOME_PRIOR` — the deliberate asymmetry vs `normalizeRelevance` diff --git a/supabase/functions/_shared/schemas/relevant.ts b/supabase/functions/_shared/schemas/relevant.ts index 1917a8af3..8d997346e 100644 --- a/supabase/functions/_shared/schemas/relevant.ts +++ b/supabase/functions/_shared/schemas/relevant.ts @@ -39,6 +39,10 @@ export const RelevantQuerySchema = z.object({ * Free-text query. Matched with Postgres `websearch` FTS over `key || value`, * the same index `POST /memories/search` uses — so a term that finds a lesson * there finds it here, and only the ORDER differs. + * + * With `q` set the candidate window is cut by lexical RANK (`ts_rank_cd`), + * not by recency, so the best matches are the ones that survive it. Without + * `q` there is no rank to cut by and the window stays `updated_at desc`. */ q: z.string().max(1000).optional(), /** @@ -60,15 +64,21 @@ export const RelevantQuerySchema = z.object({ * something weak" — which is what a per-turn hook needs, since injecting an * irrelevant lesson on every prompt is worse than injecting nothing. * - * Mind the floor this reaches today. When `q` is set every matched candidate - * carries `relevance: 1` — relevance is binary here (see the handler's - * docblock) — and the outcome factor never sinks below its cold-start prior - * of `0.5` (it maps to `1`, `0.75`, or the prior — never `0`). So with the - * endpoint's four equal weights no matched score drops below - * `(1 + 0.5) / 4 = 0.375`, and a `min_score` at or under `0.375` is therefore - * a no-op. Above `0.375` it still discriminates, on recency and salience. A - * `min_score` that gates across the whole range needs graded FTS relevance, - * which lands with the ranked-relevance RPC (PR 11). + * IT NOW GATES ACROSS THE WHOLE RANGE, and that is a behaviour change worth + * knowing about if you set it before migration 00112. Relevance used to be + * BINARY — every matched candidate carried `relevance: 1` — so with the + * endpoint's four equal weights and an outcome factor that never sinks below + * its `0.5` cold-start prior, no matched score could drop under + * `(1 + 0.5) / 4 = 0.375`, and any `min_score` at or below that was a no-op. + * + * Relevance is now the graded lexical rank (`ts_rank_cd`, normalised into + * `[0,1)`), so a faint match scores a faint relevance and the old floor is + * gone: a weak, stale, unrecurring hit can now land below `0.375` and be + * filtered. That is the point of grading — but it means a `min_score` chosen + * against the binary behaviour is now stricter than it was, and a value that + * used to pass everything may start dropping real hits. Re-tune it against + * the `score` values you actually see rather than carrying the old number + * over. */ min_score: z.coerce.number().min(0).max(1).optional().default(0), }); @@ -86,6 +96,14 @@ export const RelevantEntrySchema = z.object({ factors: z.object({ recency: z.number(), salience: z.number(), + /** + * Graded lexical relevance in `[0,1)` — Postgres `ts_rank_cd` over the + * `fts` column, saturated into the range (migration 00112). `0` means the + * request carried no `q` at all, never "matched, but badly": a row that did + * not match is not returned. It is a LEXICAL score — term overlap and + * proximity, deterministically computed — and never a semantic or + * embedding similarity. + */ relevance: z.number(), outcome: z.number(), }), diff --git a/supabase/functions/memories/CLAUDE.md b/supabase/functions/memories/CLAUDE.md index 75dc93b43..ff928a990 100644 --- a/supabase/functions/memories/CLAUDE.md +++ b/supabase/functions/memories/CLAUDE.md @@ -502,7 +502,7 @@ The one verb that **ranks**. Returns the top-K lessons for a query as a compact "key": "migration-order", "hook": "Always add the column before the backfill runs.", "score": 0.74, - "factors": { "recency": 0.61, "salience": 0.85, "relevance": 1, "outcome": 0.5 }, + "factors": { "recency": 0.61, "salience": 0.85, "relevance": 0.8, "outcome": 0.5 }, "seen_count": 9, "updated_at": "2026-07-30T09:12:00.000Z" } @@ -513,10 +513,10 @@ The one verb that **ranks**. Returns the top-K lessons for a query as a compact | Param | Default | Meaning | |-------|---------|---------| -| `q` | — | Free-text query, `websearch` FTS over `key \|\| value`. **Optional.** | +| `q` | — | Free-text query, `websearch` FTS over `key \|\| value`. **Optional.** With `q` the candidate window is cut by `ts_rank_cd` desc; without it, by `updated_at` desc. | | `scopes` | all visible | Comma-separated, **most-specific first** — the order breaks ties. | | `limit` | `10` | 1–50. A shortlist for a context window, not a page. | -| `min_score` | `0` | Drop hits below this — how a caller says "stay silent rather than show me something weak". Note: with `q` set, matched hits floor at `(1 + 0.5) / 4 = 0.375` (relevance is binary and outcome never sinks below its `0.5` prior today), so `min_score ≤ 0.375` is a no-op until graded relevance. | +| `min_score` | `0` | Drop hits below this — how a caller says "stay silent rather than show me something weak". It gates across the whole range since 00112: binary relevance floored every matched hit at `(1 + 0.5) / 4 = 0.375`, and grading removed that floor, so a value tuned against the old behaviour is now stricter than it was. | **Why it exists.** Every other read hands the caller a single-signal ordering — `GET /memories` is `updated_at` desc, `POST /memories/search` is FTS rank — and neither knows that a lesson @@ -524,9 +524,10 @@ written twelve times is worth more than one written once. Each client that wante shortlist fetched a page and re-sorted it locally, which is three copies of a ranking and three chances to disagree about what matters. -**Two phases, and the split is the design.** Postgres SELECTS the candidates (an index scan -over `fts` is the difference between reading 40 rows and reading the whole store); the shared -scorer ORDERS them in TypeScript. The ranking is deliberately *not* SQL: it is set-relative +**Two phases, and the split is the design.** Postgres SELECTS AND GRADES the candidates, +through `lorekit_memory_relevant_candidates` (migration 00112) — an index scan over `fts` is +the difference between reading 40 rows and reading the whole store; the shared scorer ORDERS +them in TypeScript. The ranking is deliberately *not* SQL: it is set-relative (salience normalises against the most-recurring candidate) and it must agree exactly with the CLI hook's ordering. A plpgsql copy could not be held to that agreement by any test, whereas `lesson-rank-parity.spec.ts` holds `_shared/ranking/lesson-rank.ts` to the CLI's `lessons-pure.mjs` @@ -537,17 +538,30 @@ matters for this task". Without it, the ranking is recency + salience, which is generally" — the SessionStart question. Requiring `q` would have forced the hook to invent a query for a session that has not asked anything yet. -**Relevance is currently binary, and that is an honest limit.** `ts_rank` is not projectable -through PostgREST's query grammar, so a row that matched scores 1 and — since a non-matching -row is never returned — nothing scores between. Ordering among matches is therefore decided by -recency and salience, which is the useful half: *these all mention your terms; here are the -ones that keep mattering.* A graded relevance needs an RPC returning `ts_rank`, which is where -the semantic-search work has to go anyway. +**Relevance is GRADED, and it is lexical.** `ts_rank` is not projectable through PostgREST's +query grammar, which is why relevance used to be binary — a row that matched scored 1, and +since a non-matching row is never returned, nothing scored between. Migration 00112 moves the +candidate fetch into an RPC that returns `ts_rank_cd`, so ordering among matches now reflects +*how well* each row matched: cover density, which rewards query terms sitting close together +over a lesson that merely mentions them in different paragraphs. The handler saturates the raw +(unbounded) value into `[0,1)` with `normalizeLexicalRank`, a strictly increasing map that +cannot reorder what Postgres ranked; `LEXICAL_RANK_SATURATION` is anchored to the default D +weight every lexeme in `memories.fts` carries, and both the constant and the anchor are pinned +(`lexical-rank.spec.ts`, `migrations.test.sql` §112 AC-4). + +It is a LEXICAL score and never a semantic one. Nothing on this path calls a model, there is no +vector index, and identical inputs produce identical output — see +[decisions.md](../../../docs/decisions.md#graded-relevance-is-lexical-ts_rank_cd-never-a-vector-score). **`CANDIDATE_LIMIT = 200`** bounds the pre-ranking fetch. The ranking needs a population, not just the page it returns — salience is normalised across candidates, so a genuinely recurring -lesson ranked 30th by FTS must still get the chance to come first. 200 is comfortably above -the route's own `limit` cap of 50 while staying one cheap indexed read. +lesson ranked 30th by relevance must still get the chance to come first. 200 is comfortably +above the route's own `limit` cap of 50 while staying one cheap indexed read. WITH a `q` the +cap is no longer an honesty problem — the RPC cuts the window in rank order, so the rows that +fall off matched worst. WITHOUT a `q` there is no rank to order by and the window is still +`updated_at desc`, so a high-`seen_count` old lesson can still miss the set; that residue is +deliberate, and the no-`q` call is the SessionStart question where recency is a defensible +window. The response carries `candidates` (how many the FTS matched before ranking) so a caller can say "3 of 47" instead of implying it saw everything — the same reason the SessionStart block diff --git a/supabase/functions/memories/handlers/relevant.ts b/supabase/functions/memories/handlers/relevant.ts index 66f9f95a4..1c2a079c1 100644 --- a/supabase/functions/memories/handlers/relevant.ts +++ b/supabase/functions/memories/handlers/relevant.ts @@ -2,22 +2,18 @@ import type { AuthContext } from '../../_shared/api/auth.ts'; import { forbidden, ok } from '../../_shared/api/respond.ts'; import { validateQuery } from '../../_shared/api/validate.ts'; import { createTracedClient } from '../../_shared/telemetry/otel.ts'; -import type { TracedQuery, Span } from '../../_shared/telemetry/otel.ts'; +import type { Span } from '../../_shared/telemetry/otel.ts'; import type { DbClient } from '../../_shared/api/auth.ts'; -import type { Tables } from '../../_shared/db/database.types.ts'; -import { getMemberOrgIds, applyRestTenantScope, firstDeniedScope } from '../../_shared/api/tenant.ts'; +import { firstDeniedScope } from '../../_shared/api/tenant.ts'; import { keyRestriction } from '../../_shared/api/auth.ts'; -import { - RelevantQuerySchema, - RELEVANT_SELECT, - lessonHook, -} from '../../_shared/schemas/relevant.ts'; +import { RelevantQuerySchema, lessonHook } from '../../_shared/schemas/relevant.ts'; import { parseTagsParam } from '../../_shared/schemas/tags.ts'; import { rankLessons, selectDiverse, recencyFactor, salienceFactor, + normalizeLexicalRank, normalizeRelevance, normalizeOutcome, seenCountFrom, @@ -26,26 +22,48 @@ import { import type { RankableLesson } from '../../_shared/ranking/lesson-rank.ts'; import { outcomeFromTags } from '../../_shared/ranking/outcome-signal.ts'; -type MemoryRow = Tables<'memories'>; +/** + * One row of `lorekit_memory_relevant_candidates` (migration 00112) — the + * `RELEVANT_SELECT` projection plus the graded `relevance`. + * + * `relevance` is the RAW, unbounded `ts_rank_cd` value, and `null` when the + * request carried no `q`. Normalising it is this handler's job, not the RPC's: + * the factor the scorer consumes has to be in [0,1], and where that mapping + * lives has to be somewhere a unit test can pin it. + */ +interface RawRelevantCandidate { + scope: string; + key: string; + value: string; + seen_count: number | null; + updated_at: string; + tags: string[] | null; + origin_pr: number | null; + relevance: number | null; +} /** - * How many rows the FTS may return before ranking. The ranking is set-relative - * — salience normalises against the most-recurring candidate — so it needs a - * population, not just the page it will return, or a genuinely recurring lesson - * ranked 30th by FTS never gets the chance to come first. + * How many rows the candidate RPC may return before ranking. The ranking is + * set-relative — salience normalises against the most-recurring candidate — so + * it needs a population, not just the page it will return, or a genuinely + * recurring lesson ranked 30th by relevance never gets the chance to come + * first. * * Bounded because the cost is real: every candidate is fetched, scored and * mostly discarded. 200 is comfortably more than any `limit` this route accepts * (50) while staying one cheap indexed read. * - * AND THE HONEST LIMIT THE BOUND BUYS: the window is cut in `updated_at desc` - * order, NOT by rank. On a store with more than `CANDIDATE_LIMIT` active rows - * matching the filters — most acutely with no `q`, where the filters are just - * "active" — an old lesson with a high `seen_count` never enters the set, so - * salience cannot surface the very row it exists for. It is a recency-windowed - * ranking, not a global one. Widening the cap only moves the cliff; removing it - * needs the candidates chosen by rank in Postgres, which is the same `ts_rank` - * RPC graded relevance needs (see the relevance note below) and belongs there. + * WHAT THE BOUND COSTS, now that migration 00112 cuts the window by rank. WITH + * a `q` the cap is no longer an honesty problem: the RPC orders by + * `ts_rank_cd` desc, so the rows that fall off the end are the ones that + * matched WORST, which is the one population it is safe to discard. WITHOUT a + * `q` there is no rank to order by and the window is still `updated_at desc`, + * so on a store with more than `CANDIDATE_LIMIT` active rows an old lesson with + * a high `seen_count` can still miss the set and salience cannot surface the + * very row it exists for. That residue is real and deliberate: a global + * salience order would need its own indexed ordering in the RPC, and the + * no-`q` call is the SessionStart question, where recency is a defensible + * window rather than an arbitrary one. */ const CANDIDATE_LIMIT = 200; @@ -60,10 +78,14 @@ const CANDIDATE_LIMIT = 200; * * TWO PHASES, and the split is the design: * - * 1. POSTGRES SELECTS the candidates. FTS decides what could possibly be - * relevant, and it is the one part that must run in the database — an - * index scan over `fts` is the difference between reading 40 rows and - * reading the tenant's entire store. + * 1. POSTGRES SELECTS AND GRADES the candidates, through + * `lorekit_memory_relevant_candidates` (migration 00112). FTS decides + * what could possibly be relevant, and it is the one part that must run in + * the database — an index scan over `fts` is the difference between + * reading 40 rows and reading the tenant's entire store. The RPC also + * returns HOW WELL each row matched (`ts_rank_cd`) and, when a `q` is + * present, cuts the candidate window in rank order rather than recency + * order. * 2. THE SHARED SCORER ORDERS them, in TypeScript, over the fetched set. * Not SQL: the ranking is set-relative (salience is normalised against the * most-recurring candidate) and it must agree exactly with the CLI hook's @@ -71,9 +93,21 @@ const CANDIDATE_LIMIT = 200; * agreement by any test, whereas `lesson-rank-parity.spec.ts` holds this * one to the CLI's `lessons-pure.mjs` behaviourally. * + * WHY AN RPC RATHER THAN THE POSTGREST QUERY IT REPLACES: `ts_rank_cd` is not + * projectable through PostgREST's query grammar, so from the PostgREST side a + * matched row could only ever score 1 and a non-matched row was never returned + * — relevance was binary, and the candidate window was cut by recency, so the + * best-matching lesson in a large store could fall outside it entirely. Both + * are properties of the SELECT, so both are fixed in the SELECT. + * * Relevance comes from the FTS side, so a query with no `q` legitimately ranks * on recency + salience alone — which is exactly the SessionStart question, and * why `q` is optional. + * + * DETERMINISTIC END TO END: `ts_rank_cd` is a pure Postgres function, the + * candidate order is a total order (rank, then `updated_at`, then `id`), and + * the scorer is pure. Identical request, identical rows, identical response — + * no model and no vector index anywhere on the path. */ export async function handleRelevant( req: Request, auth: AuthContext, db: DbClient, span: Span, @@ -99,9 +133,11 @@ export async function handleRelevant( // Early refusal for a NAMED scope outside the key's allowlist (00068/00069), // identical to `POST /memories/search`, which takes the same list shape. - // Without it `applyRestTenantScope` narrows the candidate set to empty, which - // reads as "there is nothing relevant there" rather than "you may not ask - // about that scope". EVERY named scope must be allowed, not just one: + // Without it the RPC's `lorekit_api_token_scope_allowed` predicate narrows + // the candidate set to empty, which reads as "there is nothing relevant + // there" rather than "you may not ask about that scope". The refusal is not + // redundant with that predicate — it is the difference between a 403 and a + // misleading 200. EVERY named scope must be allowed, not just one: // answering over the allowed subset would answer a different question than // the one asked, and the precedence order the caller expressed would silently // lose a rank. `firstDeniedScope` returns null for a JWT/service caller and @@ -117,57 +153,56 @@ export async function handleRelevant( const tracedDb = createTracedClient(db, span); - let q: TracedQuery = tracedDb - .from('memories') - .select(RELEVANT_SELECT) - // Active lore only — the same partition every read path applies. An - // archived or expired lesson is not a candidate for "what should I read". - .is('archived_at', null) - .or('expires_at.is.null,expires_at.gt.now()') - // A deterministic order over the CANDIDATE fetch. It is not the answer's - // order (the scorer decides that), but without it the set of rows that - // survives the cap would vary between identical requests, which would make - // the endpoint non-deterministic in a way no caller could see. - .order('updated_at', { ascending: false }) - .order('id', { ascending: false }) - .limit(CANDIDATE_LIMIT); - - // api_key auth uses the service-role client (bypasses RLS) — apply the tenant - // filter. JWT auth is RLS-scoped and needs none. Identical to every sibling - // read route; there is no second predicate here to drift from them. - if (auth.type === 'api_key' && auth.userId) { - const orgIds = await getMemberOrgIds(db, auth.userId, span); - q = applyRestTenantScope(q, auth.userId, orgIds, keyRestriction(auth)); - } - - if (params.q) q = q.textSearch('fts', params.q, { type: 'websearch', config: 'english' }); - if (scopes.length) q = q.in('scope', scopes); - - const { data, error } = await q; + // TENANCY IS RESOLVED INSIDE THE RPC, not layered on outside it. The + // PostgREST form needed `getMemberOrgIds` + `applyRestTenantScope` here + // because a service-role client bypasses RLS; the RPC composes the SAME three + // functions those helpers mirror — `lorekit_member_org_ids` (00014), + // `lorekit_api_token_org_allowed` and `lorekit_api_token_scope_allowed` + // (00068/00069) — so the predicate is asked once, in SQL, for both auth + // tiers. What travels from here is only the calling key's own restriction, + // which lives on the token and nowhere else. That also drops a round trip: + // the membership lookup is now a join inside the one query instead of a + // separate RPC before it. + const { data, error } = await tracedDb.rpc( + 'lorekit_memory_relevant_candidates', + { + p_user_id: auth.userId ?? null, + // `|| null` rather than `?? null`: an EMPTY `q` is "no query", exactly as + // the `if (params.q)` guard this replaces treated it. A non-empty query + // whose every term is a stopword still goes through the FTS predicate and + // still matches nothing — the RPC deliberately does not rescue it into + // the unfiltered list. + p_q: params.q || null, + p_scopes: scopes, + // The calling key's restriction (00068/00069), spelled exactly as every + // other RPC-backed handler spells it. All THREE are required: `p_scopes` + // narrows which scopes, `p_key_org_access`/`p_key_org_ids` narrow which + // ORGS, and each defaults to the UNRESTRICTED value in SQL — so omitting + // one does not fail, it fails OPEN. + p_key_scopes: keyRestriction(auth)?.scopes ?? [], + p_key_org_access: keyRestriction(auth)?.orgAccess ?? 'all', + p_key_org_ids: keyRestriction(auth)?.orgIds ?? [], + p_limit: CANDIDATE_LIMIT, + }, + ); if (error) { span.error(`DB: ${error.message}`); throw error; } - const rows = (data ?? []) as MemoryRow[]; - - // RELEVANCE IS BINARY HERE, AND THAT IS AN HONEST LIMIT RATHER THAN AN - // OVERSIGHT. `ts_rank` is not projectable through PostgREST's query grammar, - // so a row that matched the FTS predicate scores 1 and — since a non-matching - // row was never returned — nothing scores between. The ordering among matches - // is therefore decided by recency and salience, which is the useful half: - // "these all mention your terms, here are the ones that keep mattering". A - // graded relevance needs an RPC returning `ts_rank`, which is PR 11's - // territory (it has to happen there anyway for the semantic fusion). - const matched = Boolean(params.q); + const rows = (data ?? []) as RawRelevantCandidate[]; + + // RELEVANCE IS GRADED (migration 00112). `relevance` off the RPC is the raw + // `ts_rank_cd` — unbounded above, and `null` when no `q` was asked — so it is + // mapped onto the [0,1) factor `scoreLesson` consumes by + // `normalizeLexicalRank`, a strictly increasing map that cannot reorder what + // Postgres ranked. A `null` normalises to 0, which is byte-for-byte what the + // no-`q` path scored before grading existed. const candidates: (RankableLesson & { scope: string; key: string; value: string })[] = rows.map((r) => ({ scope: r.scope, key: r.key, value: r.value, - seen_count: (r as MemoryRow & { seen_count?: number }).seen_count ?? null, + seen_count: r.seen_count, updated_at: r.updated_at, - relevance: matched ? 1 : 0, - outcome: outcomeFromTags( - (r as MemoryRow & { tags?: string[] | null }).tags, - (r as MemoryRow & { origin_pr?: number | null }).origin_pr, - ), + relevance: normalizeLexicalRank(r.relevance), + outcome: outcomeFromTags(r.tags, r.origin_pr), })); const now = Date.now(); diff --git a/supabase/migrations/00112_relevant_ranking.sql b/supabase/migrations/00112_relevant_ranking.sql new file mode 100644 index 000000000..57f090b30 --- /dev/null +++ b/supabase/migrations/00112_relevant_ranking.sql @@ -0,0 +1,197 @@ +-- ═════════════════════════════════════════════════════════════════════════ +-- lorekit_memory_relevant_candidates — the candidate set behind +-- `GET /memories/relevant`, selected and GRADED by Postgres. +-- +-- WHAT THIS REPLACES, and why it had to become an RPC. The handler used to +-- build the candidate fetch with PostgREST: +-- +-- .textSearch('fts', q, { type: 'websearch', config: 'english' }) +-- .order('updated_at', { ascending: false }) +-- .limit(CANDIDATE_LIMIT) +-- +-- which buys two honest limits the handler documented and could not fix from +-- the TypeScript side: +-- +-- 1. RELEVANCE WAS BINARY. `ts_rank`/`ts_rank_cd` is not projectable through +-- PostgREST's query grammar, so a row that matched scored 1 and a row +-- that did not was never returned — nothing scored between. Ordering +-- among matches was decided entirely by recency and salience. +-- 2. THE CANDIDATE WINDOW WAS CUT BY RECENCY, NOT BY RANK. On a store with +-- more matching rows than the cap, the best-matching lesson could simply +-- fall outside the window and never be scored at all. +-- +-- Both are the same fix: choose and grade the candidates IN Postgres. This +-- function returns the `RELEVANT_SELECT` projection plus a `relevance` column +-- = `ts_rank_cd(fts, websearch_to_tsquery('english', p_q))`, and — when a +-- query is supplied — cuts the window in RANK order instead of recency order. +-- +-- DETERMINISTIC AND EMBEDDING-FREE, BY CONSTRUCTION. `ts_rank_cd` is a pure, +-- reproducible Postgres function over the same `fts` column `POST +-- /memories/search` already matches on. There is no model, no vector index and +-- no external call anywhere on this path: the same request against the same +-- rows returns the same rows in the same order, run to run. That is a +-- deliberate product decision, not an implementation detail — see +-- docs/decisions.md → "Graded relevance is lexical `ts_rank_cd`, never a +-- vector score". +-- +-- WHY `ts_rank_cd` AND NOT `ts_rank`. Cover density rewards query terms that +-- appear CLOSE TOGETHER, which is the signal that separates a lesson actually +-- about "migration backfill" from one that mentions migrations in its first +-- paragraph and backfills in its last. `ts_rank` counts occurrences without +-- regard to proximity and would rank the second one just as highly. +-- +-- THE SCORER STILL LIVES IN TYPESCRIPT. This function does NOT rank — it +-- SELECTS and GRADES. The composite score stays in the shared +-- `lesson-rank.ts`/`lessons-pure.mjs` pair, because it is set-relative +-- (salience normalises against the most-recurring candidate) and must agree +-- exactly with the CLI hook's ordering, an agreement `lesson-rank-parity.spec.ts` +-- can only hold two TypeScript-family implementations to. A plpgsql third copy +-- would be unguardable. The split is unchanged: Postgres selects, TypeScript +-- orders — this migration only makes the SELECT side say how well each row +-- matched instead of merely that it did. +-- +-- TENANCY IS COMPOSED, NEVER RE-DERIVED. Three predicates, each the same +-- function its TypeScript counterpart mirrors: +-- * `lorekit_member_org_ids` — the single tenant-visibility source (00014), +-- exactly as `rls_read` (00015) and every sibling analytics RPC +-- (`lorekit_memory_read_ranking` 00085, `lorekit_read_activity`) ask it. +-- * `lorekit_api_token_org_allowed` (00068) — the SQL twin of +-- `effectiveOrgIds`/`ownRowsFragment` in `tenant-scope.ts`. Membership AND +-- the key's tenancy, which is the intersection the TypeScript filter +-- computes; a personal row (`org_id is null`) stays reachable under every +-- tenancy, and the owner's OWN org-owned rows drop out under a restricted +-- key — the case a bare `user_id = v_actor` disjunct would wrongly admit. +-- * `lorekit_api_token_scope_allowed` (00068/00069) — the SQL twin of +-- `keyScopeFilter`. A malformed stored pattern is dropped on both sides, +-- so a key can only ever be narrowed by one. +-- `security definer` (00085's shape) rather than invoker: the api_key tier +-- calls this on the SERVICE-ROLE client, where invoker would mean no RLS and +-- no predicate at all. Definer gives ONE predicate serving both auth tiers +-- instead of a predicate for one and RLS for the other. +-- +-- INDEXES: no new index. The `q` path is an `fts @@ tsquery` match served by +-- `memories_fts_idx` (00001, GIN); the no-`q` path is `updated_at desc, id +-- desc` served by `memories_user_updated_at_id_idx` (00096). `ts_rank_cd` is +-- evaluated only over rows the index already returned — it is a recheck on a +-- bounded set, never a scan driver. +-- ═════════════════════════════════════════════════════════════════════════ + +create or replace function lorekit_memory_relevant_candidates( + p_user_id uuid, + p_q text default null, + p_scopes text[] default '{}', + p_key_scopes text[] default '{}', + p_key_org_access text default 'all', + p_key_org_ids uuid[] default '{}', + p_limit integer default 200 +) +returns table ( + scope text, + key text, + value text, + seen_count integer, + updated_at timestamptz, + tags text[], + origin_pr integer, + relevance double precision +) +language plpgsql +stable +security definer +set search_path = public +as $$ +#variable_conflict use_column +declare + v_actor uuid := case + when auth.role() = 'service_role' then coalesce(p_user_id, auth.uid()) + else auth.uid() + end; + -- Clamped the same way every sibling analytics RPC clamps: the caller names + -- the candidate cap, but the cap is this function's cost ceiling, not the + -- caller's to raise without bound. + v_limit integer := least(greatest(coalesce(p_limit, 200), 1), 1000); + -- NULL and '' are BOTH "no query", matching the handler's `if (params.q)` + -- falsiness exactly so the pre-RPC behaviour is preserved byte for byte. + -- + -- A non-empty `p_q` whose tsquery comes back EMPTY (every term a stopword, + -- e.g. 'the') is deliberately NOT folded into the no-query path: an empty + -- tsquery matches nothing, so such a request returns no candidates — which + -- is exactly what the PostgREST `.textSearch` form did. Folding it into "no + -- query" would silently turn a request that asked a question into the + -- unfiltered recency list, which is a different answer, not a better one. + v_query tsquery := case + when p_q is null or p_q = '' then null + else websearch_to_tsquery('english', p_q) + end; +begin + return query + select + m.scope, + m.key, + m.value, + m.seen_count, + m.updated_at, + m.tags, + m.origin_pr, + -- NULL when no query was asked — the absence of a relevance opinion, + -- which the handler maps to 0 exactly as the binary form did. `greatest( + -- …, 0)` is a floor, not a clamp: ts_rank_cd is non-negative by + -- definition, and the floor documents that an authorization-adjacent + -- numeric never leaves this function negative. + case + when v_query is null then null::double precision + else greatest(ts_rank_cd(m.fts, v_query), 0)::double precision + end as relevance + from memories m + where ( + -- Service-role with no actor (CI) reads everything, the same + -- escape hatch 00085 carries. + (v_actor is null and auth.role() = 'service_role') + or m.user_id = v_actor + or m.org_id in (select lorekit_member_org_ids(v_actor)) + ) + -- Active lore only — the partition every read path applies. An archived + -- or expired lesson is not a candidate for "what should I read". + and m.archived_at is null + and (m.expires_at is null or m.expires_at > now()) + and lorekit_api_token_org_allowed( + coalesce(p_key_org_access, 'all'), + coalesce(p_key_org_ids, '{}'::uuid[]), + m.org_id + ) + and lorekit_api_token_scope_allowed(p_key_scopes, m.scope) + and (coalesce(cardinality(p_scopes), 0) = 0 or m.scope = any(p_scopes)) + and (v_query is null or m.fts @@ v_query) + order by + -- RANK FIRST when there is a rank to sort by — this is the half that + -- closes the "recency-windowed, not global" caveat. With no query every + -- row's sort key is NULL, so the expression collapses and the ordering + -- is the pre-RPC `updated_at desc, id desc` exactly. + case when v_query is null then null else ts_rank_cd(m.fts, v_query) end + desc nulls last, + m.updated_at desc, + -- The total-order tie-break. Without it the set that survives the cap + -- could differ between two identical requests, which is the one way + -- this endpoint could be non-deterministic without any caller being + -- able to see why. + m.id desc + limit v_limit; +end; +$$; + +revoke execute on function lorekit_memory_relevant_candidates(uuid, text, text[], text[], text, uuid[], integer) + from public, anon; +grant execute on function lorekit_memory_relevant_candidates(uuid, text, text[], text[], text, uuid[], integer) + to authenticated, service_role; + +comment on function lorekit_memory_relevant_candidates(uuid, text, text[], text[], text, uuid[], integer) is + 'Candidate rows for GET /memories/relevant, graded by ts_rank_cd over the + memories.fts column. With p_q set the window is cut in RANK order (closing + the recency-window caveat) and relevance is the raw, unbounded ts_rank_cd + value -- the caller normalises it into [0,1]. With p_q null or empty the + window is cut in updated_at desc order and relevance is NULL. Active rows + only (non-archived, non-expired). Visibility is lorekit_member_org_ids + narrowed by lorekit_api_token_org_allowed and lorekit_api_token_scope_allowed, + so it can never be wider than the RLS read policy or the calling key. + Deterministic and embedding-free: no model, no vector index, no external + call. p_limit is clamped to [1, 1000].'; diff --git a/supabase/tests/migrations.test.sql b/supabase/tests/migrations.test.sql index ffc82c5b2..51841213e 100644 --- a/supabase/tests/migrations.test.sql +++ b/supabase/tests/migrations.test.sql @@ -10277,6 +10277,350 @@ begin end; $$; +-- ═════════════════════════════════════════════════════════════════════════ +-- §112 — lorekit_memory_relevant_candidates: graded lexical relevance and a +-- rank-cut candidate window (00112) +-- +-- `GET /memories/relevant` now selects its candidates through this function. +-- Before it, relevance was BINARY (a row matched or it did not, because +-- ts_rank_cd is not projectable through PostgREST's query grammar) and the +-- candidate window was cut in `updated_at desc` order, so the best-matching +-- lesson in a large store could fall outside the window entirely. Both are +-- properties of the SELECT, so both are asserted here rather than in a handler +-- spec that can only mock what the SELECT returned. +-- +-- Fresh users and a fresh org, so every assertion is an exact set rather than a +-- `like` filter over whatever earlier sections left behind. +-- +-- THE ASSERTIONS ARE RELATIVE, NOT ABSOLUTE, wherever a `ts_rank_cd` magnitude +-- would otherwise be hard-coded. What this migration promises is an ORDER — +-- "better matches rank higher, and the window keeps the better matches" — not +-- particular numbers, and a test that pinned the numbers would fail on a +-- Postgres ranking-internals change that broke nothing anybody relies on. The +-- one exception is AC-4, which pins a magnitude on purpose because a TypeScript +-- constant is derived from it. +-- +-- AC-1: relevance is GRADED — a row matching two terms of an OR query outranks +-- one matching a single term, and the returned order IS rank order. +-- AC-2: relevance is graded by density too — repeating the query term ranks +-- above a single occurrence of it. +-- AC-3: the candidate window is cut by RANK, not recency. A 1-row window holds +-- the best match, not the most recently updated match. This is the +-- "recency-windowed, not global" caveat the handler used to document. +-- AC-4: the saturation anchor the TypeScript normalizer is pinned to is real — +-- one occurrence of one default-weight lexeme is a ts_rank_cd of ≈ 0.1. +-- AC-5: p_q null is the no-query path — relevance is NULL on every row and the +-- order is `updated_at desc`, byte-for-byte the pre-00112 behaviour. An +-- empty string is the same "no query", matching the handler's falsiness. +-- AC-6: an all-stopword query is NOT rescued into the no-query path — an empty +-- tsquery matches nothing, exactly as the PostgREST `.textSearch` form +-- behaved. Silently answering a different question would be worse. +-- AC-7: archived and expired rows are never candidates. +-- AC-8: tenancy — another user's rows are invisible however well they match; an +-- org-shared row is visible to a member. +-- AC-9: the calling key's own restrictions narrow further — `personal` tenancy +-- drops org rows the OWNER can otherwise see, `selected` admits the named +-- org, and a scope allowlist narrows a call that names no scope. +-- AC-10: determinism — two identical calls return the identical order. +-- ═════════════════════════════════════════════════════════════════════════ + +insert into auth.users (id) values + ('11200000-0000-0000-0000-000000000001'), + ('11200000-0000-0000-0000-000000000002') + on conflict do nothing; + +insert into orgs (id, slug, name, created_by) values + ('11200000-0000-0000-0000-0000000000f1', 'relv-org', 'Relevance Org', + '11200000-0000-0000-0000-000000000001') + on conflict do nothing; +insert into org_members (org_id, user_id, role) values + ('11200000-0000-0000-0000-0000000000f1', '11200000-0000-0000-0000-000000000001', 'owner') + on conflict do nothing; + +do $$ +declare + v_owner uuid := '11200000-0000-0000-0000-000000000001'; + v_other uuid := '11200000-0000-0000-0000-000000000002'; + v_org uuid := '11200000-0000-0000-0000-0000000000f1'; + v_keys text[]; + v_ranks double precision[]; + v_rank double precision; + v_rank_one double precision; + v_rank_two double precision; + v_best double precision; + v_nulls bigint; + v_rows bigint; + v_first text; + v_run_a text[]; + v_run_b text[]; + i integer; +begin + -- ── Fixtures ──────────────────────────────────────────────────────────── + -- `both` carries BOTH query terms, `one` carries a single one, `dense` + -- repeats its single term, `single` holds exactly one occurrence of it (the + -- AC-4 probe). `fresh` is the recency decoy: a weak single-term match that is + -- the most recently updated MATCHING row, so a recency-cut window would keep + -- it and drop the better matches — which is precisely what AC-3 rejects. + -- + -- Every `updated_at` is explicit. The no-query path orders by it, so leaving + -- any of them to `now()` would make AC-5c depend on the wall clock. + insert into memories (user_id, scope, key, value, updated_at) values + (v_owner, 'project::relv', '112-both', + 'migration backfill order matters here', timestamptz '2026-01-01'), + (v_owner, 'project::relv', '112-one', + 'migration order matters here and nothing else does', timestamptz '2026-01-02'), + (v_owner, 'project::relv', '112-dense', + 'migration migration migration', timestamptz '2026-01-03'), + (v_owner, 'project::relv', '112-single', + 'migration', timestamptz '2026-01-04'), + (v_owner, 'project::relv', '112-fresh', + 'backfill', timestamptz '2026-06-01'); + -- Never candidates: archived, and expired. + insert into memories (user_id, scope, key, value, updated_at, archived_at) values + (v_owner, 'project::relv', '112-archived', 'migration backfill', + timestamptz '2026-01-05', now()); + insert into memories (user_id, scope, key, value, updated_at, expires_at) values + (v_owner, 'project::relv', '112-expired', 'migration backfill', + timestamptz '2026-01-06', now() - interval '1 day'); + -- A different tenant's row, lexically identical to the best match — the only + -- honest way to prove the tenancy predicate is doing the work rather than the + -- ranking accidentally burying it. + insert into memories (user_id, scope, key, value, updated_at) values + (v_other, 'project::relv', '112-foreign', + 'migration backfill order matters here', timestamptz '2026-01-07'); + -- An org-shared row the owner reaches through MEMBERSHIP, not ownership. + -- Deliberately a single-term match so it cannot tie with `112-both` for the + -- top rank and make AC-1c's ordering assertion depend on a tiebreak. + insert into memories (user_id, org_id, scope, key, value, updated_at) values + (v_other, v_org, 'project::relv', '112-org', 'org shared migration note', + timestamptz '2026-07-01'); + + set local role service_role; + perform set_config('request.jwt.claims', + '{"sub":"11200000-0000-0000-0000-000000000001","role":"service_role"}', true); + + -- ── AC-1: graded — two matched terms outrank one ──────────────────────── + select relevance into v_rank_two from lorekit_memory_relevant_candidates( + p_user_id => v_owner, p_q => 'migration or backfill', + p_scopes => array['project::relv']) + where key = '112-both'; + select relevance into v_rank_one from lorekit_memory_relevant_candidates( + p_user_id => v_owner, p_q => 'migration or backfill', + p_scopes => array['project::relv']) + where key = '112-one'; + assert v_rank_two is not null and v_rank_one is not null, + '112 AC-1a: a matched row must carry a relevance, not NULL, when p_q is set'; + assert v_rank_two > v_rank_one, + format('112 AC-1b: a row matching BOTH query terms must outrank one matching a ' + 'single term (both=%s, one=%s) — a binary relevance scored these equal, ' + 'which is the regression 00112 exists to fix', v_rank_two, v_rank_one); + + -- The returned order must BE rank order, or the window the handler caps would + -- not be the best-matching window. Asserted as "non-increasing down the + -- result" rather than a hard-coded key sequence: the contract is the ordering + -- rule, not one particular permutation of these fixtures. + select array_agg(relevance order by ord) into v_ranks from ( + select relevance, row_number() over () as ord + from lorekit_memory_relevant_candidates( + p_user_id => v_owner, p_q => 'migration or backfill', + p_scopes => array['project::relv']) + ) t; + assert array_length(v_ranks, 1) >= 2, + format('112 AC-1c: the ordering assertion needs at least two matched rows, got %s', + coalesce(array_length(v_ranks, 1), 0)); + for i in 2 .. array_length(v_ranks, 1) loop + assert v_ranks[i] <= v_ranks[i - 1], + format('112 AC-1c: candidates must be returned in ts_rank_cd desc order — ' + 'position %s (%s) outranks position %s (%s); full sequence %s', + i, v_ranks[i], i - 1, v_ranks[i - 1], v_ranks); + end loop; + + -- ── AC-2: graded by density — repetition outranks a single occurrence ─── + select relevance into v_rank_two from lorekit_memory_relevant_candidates( + p_user_id => v_owner, p_q => 'migration', p_scopes => array['project::relv']) + where key = '112-dense'; + select relevance into v_rank_one from lorekit_memory_relevant_candidates( + p_user_id => v_owner, p_q => 'migration', p_scopes => array['project::relv']) + where key = '112-single'; + assert v_rank_two > v_rank_one, + format('112 AC-2: three occurrences of the query term must outrank one ' + '(dense=%s, single=%s)', v_rank_two, v_rank_one); + + -- ── AC-3: the window is cut by RANK, not recency ──────────────────────── + -- `112-fresh` is the most recently updated MATCHING row and a weak match. + -- Under the pre-00112 `updated_at desc` window with a cap of 1 it was the + -- only candidate; under a rank-cut window it is among the first to fall off. + select max(relevance) into v_best from lorekit_memory_relevant_candidates( + p_user_id => v_owner, p_q => 'migration or backfill', + p_scopes => array['project::relv']); + select key, relevance into v_first, v_rank from lorekit_memory_relevant_candidates( + p_user_id => v_owner, p_q => 'migration or backfill', + p_scopes => array['project::relv'], p_limit => 1); + assert v_rank = v_best, + format('112 AC-3a: a 1-row window must hold the BEST-matching row (kept %s at %s, ' + 'best in the full set is %s)', v_first, v_rank, v_best); + assert v_first <> '112-fresh', + '112 AC-3b: a 1-row window must not simply keep the most recently updated match — ' + 'that is the recency-windowed behaviour 00112 replaces'; + + -- ── AC-4: the saturation anchor the TS normalizer is pinned to ────────── + -- `memories.fts` applies no setweight (00001), so every lexeme carries + -- Postgres's default D weight of 0.1, and one occurrence of one query lexeme + -- is a ts_rank_cd of 0.1. `LEXICAL_RANK_SATURATION` in `lesson-rank.ts` IS + -- that number — this is the assertion that keeps its docblock's claim true, + -- and the one place in this section where pinning a magnitude is the point. + select relevance into v_rank from lorekit_memory_relevant_candidates( + p_user_id => v_owner, p_q => 'migration', p_scopes => array['project::relv']) + where key = '112-single'; + assert v_rank between 0.09 and 0.11, + format('112 AC-4: one occurrence of one default-weight lexeme must rank ≈ 0.1 — ' + 'the value LEXICAL_RANK_SATURATION is anchored to — got %s', v_rank); + + -- ── AC-5: the no-query path is unchanged ──────────────────────────────── + select count(*) filter (where relevance is null), count(*) + into v_nulls, v_rows + from lorekit_memory_relevant_candidates( + p_user_id => v_owner, p_q => null, p_scopes => array['project::relv']); + assert v_rows = 6, + format('112 AC-5a: with no query every ACTIVE visible row is a candidate ' + '(5 own + 1 org-shared), got %s', v_rows); + assert v_nulls = v_rows, + format('112 AC-5b: with no query relevance must be NULL on every row, got %s of %s', + v_nulls, v_rows); + + select array_agg(key order by ord) into v_keys from ( + select key, row_number() over () as ord + from lorekit_memory_relevant_candidates( + p_user_id => v_owner, p_q => null, p_scopes => array['project::relv'], p_limit => 2) + ) t; + assert v_keys = array['112-org', '112-fresh'], + format('112 AC-5c: with no query the window is updated_at desc — 112-org (2026-07-01) ' + 'then 112-fresh (2026-06-01); got %s', v_keys); + + select count(*) filter (where relevance is null) into v_nulls + from lorekit_memory_relevant_candidates( + p_user_id => v_owner, p_q => '', p_scopes => array['project::relv']); + assert v_nulls = 6, + format('112 AC-5d: an empty p_q is "no query", not a query that matches nothing, ' + 'got %s rows with a null relevance', v_nulls); + + -- ── AC-6: an all-stopword query matches nothing, and is NOT rescued ───── + select count(*) into v_rows from lorekit_memory_relevant_candidates( + p_user_id => v_owner, p_q => 'the', p_scopes => array['project::relv']); + assert v_rows = 0, + format('112 AC-6: an all-stopword query yields an empty tsquery and must match ' + 'nothing — folding it into the no-query path would answer a different ' + 'question than the one asked; got %s rows', v_rows); + + -- ── AC-7: archived and expired rows are never candidates ──────────────── + assert not exists ( + select 1 from lorekit_memory_relevant_candidates( + p_user_id => v_owner, p_q => 'migration', p_scopes => array['project::relv']) + where key in ('112-archived', '112-expired') + ), '112 AC-7: an archived or expired row must never be a candidate'; + + -- ── AC-8: tenancy — foreign rows out, org-shared rows in ──────────────── + assert not exists ( + select 1 from lorekit_memory_relevant_candidates( + p_user_id => v_owner, p_q => 'migration', p_scopes => array['project::relv']) + where key = '112-foreign' + ), '112 AC-8a: another user''s row must never be a candidate, however well it matches'; + assert exists ( + select 1 from lorekit_memory_relevant_candidates( + p_user_id => v_owner, p_q => 'migration', p_scopes => array['project::relv']) + where key = '112-org' + ), '112 AC-8b: an org-shared row must be a candidate for a member of that org'; + + -- ── AC-9: the calling key's own restrictions narrow further ───────────── + -- `personal` tenancy drops every org row — including ones the OWNER reaches + -- through membership. This is the half a bare `user_id = actor` disjunct + -- would get wrong, and the half `ownRowsFragment` encodes on the TS side. + assert not exists ( + select 1 from lorekit_memory_relevant_candidates( + p_user_id => v_owner, p_q => 'migration', p_scopes => array['project::relv'], + p_key_org_access => 'personal') + where key = '112-org' + ), '112 AC-9a: a personal-tenancy key must not reach an org-shared row'; + assert exists ( + select 1 from lorekit_memory_relevant_candidates( + p_user_id => v_owner, p_q => 'migration', p_scopes => array['project::relv'], + p_key_org_access => 'personal') + where key = '112-both' + ), '112 AC-9b: a personal-tenancy key must still reach the owner''s own personal rows'; + assert exists ( + select 1 from lorekit_memory_relevant_candidates( + p_user_id => v_owner, p_q => 'migration', p_scopes => array['project::relv'], + p_key_org_access => 'selected', p_key_org_ids => array[v_org]) + where key = '112-org' + ), '112 AC-9c: a selected-tenancy key naming the org must reach that org''s rows'; + + assert not exists ( + select 1 from lorekit_memory_relevant_candidates( + p_user_id => v_owner, p_q => 'migration', + p_key_scopes => array['repo::acme/other']) + where key like '112-%' + ), '112 AC-9d: an out-of-allowlist key_scopes must narrow the candidate set to nothing, ' + 'even on a call that names no scope at all'; + + -- ── AC-10: determinism ────────────────────────────────────────────────── + select array_agg(key order by ord) into v_run_a from ( + select key, row_number() over () as ord + from lorekit_memory_relevant_candidates( + p_user_id => v_owner, p_q => 'migration or backfill', + p_scopes => array['project::relv']) + ) t; + select array_agg(key order by ord) into v_run_b from ( + select key, row_number() over () as ord + from lorekit_memory_relevant_candidates( + p_user_id => v_owner, p_q => 'migration or backfill', + p_scopes => array['project::relv']) + ) t; + assert v_run_a = v_run_b, + format('112 AC-10: two identical calls must return the identical order (%s vs %s)', + v_run_a, v_run_b); + + reset role; + perform set_config('request.jwt.claims', '', true); +end; +$$; + +-- ── 112b: the function's own shape ────────────────────────────────────────── +-- STABLE (not VOLATILE) so the planner may fold it, and SECURITY DEFINER with a +-- pinned `search_path` — the shape every sibling analytics RPC carries. A +-- definer function whose search_path is NOT pinned is a privilege-escalation +-- surface, so this is asserted rather than assumed, and `anon` must not reach +-- a function that reads memories at all. +do $$ +declare + v_sig text := 'lorekit_memory_relevant_candidates(uuid, text, text[], text[], text, uuid[], integer)'; + v_volatile "char"; + v_secdef boolean; + v_config text[]; +begin + select provolatile, prosecdef, proconfig + into v_volatile, v_secdef, v_config + from pg_proc p join pg_namespace n on n.oid = p.pronamespace + where n.nspname = 'public' and p.proname = 'lorekit_memory_relevant_candidates'; + + assert v_volatile = 's', + format('112b: lorekit_memory_relevant_candidates must be STABLE, got provolatile=%s', + v_volatile); + assert v_secdef, + '112b: the function must be SECURITY DEFINER — the api_key tier calls it on the ' + 'service-role client, where an invoker function would apply no RLS and no ' + 'predicate at all'; + assert v_config @> array['search_path=public'], + format('112b: a SECURITY DEFINER function must pin its search_path, got %s', v_config); + + assert has_function_privilege('authenticated', v_sig, 'EXECUTE'), + '112b: authenticated must be able to execute the candidate RPC (the JWT tier)'; + assert has_function_privilege('service_role', v_sig, 'EXECUTE'), + '112b: service_role must be able to execute the candidate RPC (the api_key tier)'; + assert not has_function_privilege('anon', v_sig, 'EXECUTE'), + '112b: anon must not be able to execute a function that reads memories'; +end; +$$; + rollback; \echo 'migrations.test.sql: all assertions passed' From 2c033588de8cb3e1eae52c7404ca2d5165e92c1e Mon Sep 17 00:00:00 2001 From: "mads.thines" Date: Sun, 11 Oct 2026 05:07:28 +0000 Subject: [PATCH 2/9] address review comment: docs(relevant): fix the example score to match its factors Addresses the pr-reviewer bot's comment: https://github.com/mthines/lorekit/pull/680#discussion_r4240330383 Refs: https://github.com/mthines/lorekit/pull/680 Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> --- supabase/functions/memories/CLAUDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/supabase/functions/memories/CLAUDE.md b/supabase/functions/memories/CLAUDE.md index ff928a990..d5b0d9f11 100644 --- a/supabase/functions/memories/CLAUDE.md +++ b/supabase/functions/memories/CLAUDE.md @@ -501,7 +501,7 @@ The one verb that **ranks**. Returns the top-K lessons for a query as a compact "scope": "repo::acme/app", "key": "migration-order", "hook": "Always add the column before the backfill runs.", - "score": 0.74, + "score": 0.69, "factors": { "recency": 0.61, "salience": 0.85, "relevance": 0.8, "outcome": 0.5 }, "seen_count": 9, "updated_at": "2026-07-30T09:12:00.000Z" From 80ed6cb1e66691c9b4b77cd1ea74cbf549f07452 Mon Sep 17 00:00:00 2001 From: "mads.thines" Date: Sun, 11 Oct 2026 05:12:38 +0000 Subject: [PATCH 3/9] address review comment: refactor(relevant): drop the orphaned RELEVANT_SELECT projection Addresses the pr-reviewer bot's comment: https://github.com/mthines/lorekit/pull/680#discussion_r4240330384 Refs: https://github.com/mthines/lorekit/pull/680 Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> --- packages/schemas/src/shared/relevant.ts | 9 --------- supabase/functions/_shared/schemas/relevant.ts | 9 --------- supabase/functions/memories/handlers/relevant.ts | 7 ++++++- supabase/migrations/00112_relevant_ranking.sql | 3 ++- 4 files changed, 8 insertions(+), 20 deletions(-) diff --git a/packages/schemas/src/shared/relevant.ts b/packages/schemas/src/shared/relevant.ts index ae8517b0d..1c5a29a08 100644 --- a/packages/schemas/src/shared/relevant.ts +++ b/packages/schemas/src/shared/relevant.ts @@ -134,15 +134,6 @@ export const RelevantResponseSchema = z.object({ }); export type RelevantResponse = z.infer; -/** - * The projection this route selects. Deliberately NOT `MEMORY_SELECT`: that one - * carries `value` in full plus the `orgs(...)` embed, and this route reads the - * value only to derive a one-line hook from it. Selecting the wide row for - * every candidate — of which there may be many times the returned limit — would - * move a lot of text out of Postgres to throw almost all of it away. - */ -export const RELEVANT_SELECT = 'scope,key,value,seen_count,updated_at,tags,origin_pr'; - /** Cap on a lesson's one-line hook. Matches the CLI hook's `HOOK_LEN`. */ export const RELEVANT_HOOK_LEN = 80; diff --git a/supabase/functions/_shared/schemas/relevant.ts b/supabase/functions/_shared/schemas/relevant.ts index 8d997346e..93121dd7d 100644 --- a/supabase/functions/_shared/schemas/relevant.ts +++ b/supabase/functions/_shared/schemas/relevant.ts @@ -139,15 +139,6 @@ export const RelevantResponseSchema = z.object({ }); export type RelevantResponse = z.infer; -/** - * The projection this route selects. Deliberately NOT `MEMORY_SELECT`: that one - * carries `value` in full plus the `orgs(...)` embed, and this route reads the - * value only to derive a one-line hook from it. Selecting the wide row for - * every candidate — of which there may be many times the returned limit — would - * move a lot of text out of Postgres to throw almost all of it away. - */ -export const RELEVANT_SELECT = 'scope,key,value,seen_count,updated_at,tags,origin_pr'; - /** Cap on a lesson's one-line hook. Matches the CLI hook's `HOOK_LEN`. */ export const RELEVANT_HOOK_LEN = 80; diff --git a/supabase/functions/memories/handlers/relevant.ts b/supabase/functions/memories/handlers/relevant.ts index 1c2a079c1..cba61ee3f 100644 --- a/supabase/functions/memories/handlers/relevant.ts +++ b/supabase/functions/memories/handlers/relevant.ts @@ -24,7 +24,12 @@ import { outcomeFromTags } from '../../_shared/ranking/outcome-signal.ts'; /** * One row of `lorekit_memory_relevant_candidates` (migration 00112) — the - * `RELEVANT_SELECT` projection plus the graded `relevance`. + * route's narrow projection plus the graded `relevance`. The RPC's + * `returns table` is the route's only column list. It is deliberately NOT + * `MEMORY_SELECT`'s wide row (full `value` plus the `orgs(...)` embed): the + * handler reads `value` only to derive a one-line hook, and candidates + * outnumber the returned limit, so the wide row would move text out of + * Postgres only to discard it. * * `relevance` is the RAW, unbounded `ts_rank_cd` value, and `null` when the * request carried no `q`. Normalising it is this handler's job, not the RPC's: diff --git a/supabase/migrations/00112_relevant_ranking.sql b/supabase/migrations/00112_relevant_ranking.sql index 57f090b30..e8c58aff2 100644 --- a/supabase/migrations/00112_relevant_ranking.sql +++ b/supabase/migrations/00112_relevant_ranking.sql @@ -21,7 +21,8 @@ -- fall outside the window and never be scored at all. -- -- Both are the same fix: choose and grade the candidates IN Postgres. This --- function returns the `RELEVANT_SELECT` projection plus a `relevance` column +-- function returns the route's narrow projection (`scope, key, value, +-- seen_count, updated_at, tags, origin_pr`) plus a `relevance` column -- = `ts_rank_cd(fts, websearch_to_tsquery('english', p_q))`, and — when a -- query is supplied — cuts the window in RANK order instead of recency order. -- From b8df58aaf0bbc48d7269aa8bf189c08abfa2be11 Mon Sep 17 00:00:00 2001 From: "mads.thines" Date: Sun, 11 Oct 2026 05:13:18 +0000 Subject: [PATCH 4/9] address review comment: test(relevant): pin that an authenticated caller's p_user_id is ignored Addresses the pr-reviewer bot's comment: https://github.com/mthines/lorekit/pull/680#discussion_r4240330386 Refs: https://github.com/mthines/lorekit/pull/680 Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> --- supabase/tests/migrations.test.sql | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/supabase/tests/migrations.test.sql b/supabase/tests/migrations.test.sql index 51841213e..723c4b274 100644 --- a/supabase/tests/migrations.test.sql +++ b/supabase/tests/migrations.test.sql @@ -10579,6 +10579,28 @@ begin format('112 AC-10: two identical calls must return the identical order (%s vs %s)', v_run_a, v_run_b); + -- ── AC-11: off service_role, p_user_id is ignored — auth.uid() is the actor + -- An authenticated caller that names ANOTHER user's id must still see only + -- what its own JWT can see. The negative (AC-11a) alone would pass on a call + -- that returned nothing at all, so AC-11b pins the positive twin: the + -- caller's own row stays reachable under the same spoofed call. + reset role; + set local role authenticated; + perform set_config('request.jwt.claims', + format('{"sub":"%s","role":"authenticated"}', v_owner), true); + assert not exists ( + select 1 from lorekit_memory_relevant_candidates( + p_user_id => v_other, p_q => 'migration', p_scopes => array['project::relv']) + where key = '112-foreign' + ), '112 AC-11a: off service_role a spoofed p_user_id must be ignored — ' + 'another user''s row must not become reachable'; + assert exists ( + select 1 from lorekit_memory_relevant_candidates( + p_user_id => v_other, p_q => 'migration', p_scopes => array['project::relv']) + where key = '112-both' + ), '112 AC-11b: off service_role the actor is auth.uid(), so the caller''s own ' + 'rows must stay reachable under a spoofed p_user_id'; + reset role; perform set_config('request.jwt.claims', '', true); end; From 63f18cf706fb0fc7d90a8104ff6590340636dca3 Mon Sep 17 00:00:00 2001 From: "mads.thines" Date: Sun, 11 Oct 2026 05:13:37 +0000 Subject: [PATCH 5/9] address review comment: test(tenant-scope): drop the "unlike list" clause the guard contradicts Addresses the pr-reviewer bot's comment: https://github.com/mthines/lorekit/pull/680#discussion_r4240330387 Refs: https://github.com/mthines/lorekit/pull/680 Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> --- packages/mcp-core/src/auth/tenant-scope-usage.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/mcp-core/src/auth/tenant-scope-usage.spec.ts b/packages/mcp-core/src/auth/tenant-scope-usage.spec.ts index 4b0c419d4..2c639f7e6 100644 --- a/packages/mcp-core/src/auth/tenant-scope-usage.spec.ts +++ b/packages/mcp-core/src/auth/tenant-scope-usage.spec.ts @@ -129,8 +129,8 @@ describe('tenant-scope usage guard (edge read handlers)', () => { * candidate fetch into `lorekit_memory_relevant_candidates` so Postgres could * grade it with `ts_rank_cd`, which took the query out of * `applyRestTenantScope`'s reach exactly as 00067 did for `list`. It is pinned - * by `RPC_BACKED_ROW_READS` below — AND by the tenancy guard after it, because - * unlike `list` it is a read whose visibility WIDENS to org rows. + * by `RPC_BACKED_ROW_READS` below — AND, like `list`, by the tenancy guard + * after it, because it is a read whose visibility WIDENS to org rows. */ const REST_READ_HANDLERS = ['get', 'search'] as const; From b1dbd4c85adb70521b103f884424efd3c670fb27 Mon Sep 17 00:00:00 2001 From: "mads.thines" Date: Sun, 11 Oct 2026 05:33:02 +0000 Subject: [PATCH 6/9] address review comment: docs(ranking): describe 0.1 as one cover's ceiling, not a match's floor Addresses the pr-reviewer bot's comment: https://github.com/mthines/lorekit/pull/680#discussion_r4240386682 Refs: https://github.com/mthines/lorekit/pull/680 Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> --- packages/mcp-core/src/ranking/lesson-rank.ts | 17 +++++++++++------ .../mcp-core/src/ranking/lexical-rank.spec.ts | 4 ++-- .../functions/_shared/ranking/lesson-rank.ts | 17 +++++++++++------ 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/packages/mcp-core/src/ranking/lesson-rank.ts b/packages/mcp-core/src/ranking/lesson-rank.ts index 9c976acd8..cb33ae3f9 100644 --- a/packages/mcp-core/src/ranking/lesson-rank.ts +++ b/packages/mcp-core/src/ranking/lesson-rank.ts @@ -161,14 +161,19 @@ export function normalizeRelevance(value: unknown): number { * NOT an arbitrary tuning knob — it is anchored to a fact about the schema. * `memories.fts` is `to_tsvector('english', key || ' ' || value)` with no * `setweight` call anywhere (migration 00001), so every lexeme in it carries - * Postgres's default D weight, which is `0.1`. A cover containing exactly one - * query lexeme therefore contributes `0.1` to `ts_rank_cd`, and that is the - * weakest thing a real match can be. Putting the saturation point THERE makes - * "matched about as well as a single term does" read as the midpoint of the - * factor, with denser and multi-term covers ranging above it. + * Postgres's default D weight, which is `0.1`. `ts_rank_cd` sums, over the + * covers it finds, each cover's mean lexeme weight divided by `1 +` the + * number of non-query words inside it, so one TIGHT cover — a single query + * lexeme, or query terms sitting side by side — contributes exactly `0.1`. + * That is one cover's ceiling, not a floor on a match: a multi-term cover + * whose terms sit apart scores below it, a negation-only query scores `0` on + * the rows it matches, and several covers add up above it. Putting the + * saturation point THERE makes "matched about as well as a single term + * does" read as the midpoint of the factor, with looser covers below it and + * denser, multi-cover matches above. * * The obvious alternative — saturating at `1` — is monotone and bounded too, - * but it maps the entire plausible `ts_rank_cd` range (roughly 0.1 to 1) into + * but it maps the typical `ts_rank_cd` range (roughly 0.1 to 1) into * `[0.09, 0.5]`, so a graded signal would arrive squashed into the bottom half * of its own scale and lose most of its say in the weighted average. Both * choices are deterministic; this one spends the range on values that occur. diff --git a/packages/mcp-core/src/ranking/lexical-rank.spec.ts b/packages/mcp-core/src/ranking/lexical-rank.spec.ts index 823f19d00..b6b691219 100644 --- a/packages/mcp-core/src/ranking/lexical-rank.spec.ts +++ b/packages/mcp-core/src/ranking/lexical-rank.spec.ts @@ -31,8 +31,8 @@ describe('normalizeLexicalRank: the pinned mapping', () => { }); it('maps a single-term cover onto the midpoint of the factor', () => { - // r = 0.1 is one query lexeme in one cover — the weakest a real match can - // be — and it reads 0.5, not 0.09. + // r = 0.1 is one tight cover (a single query lexeme) — the most one cover + // can score — and it reads 0.5, not 0.09. expect(normalizeLexicalRank(0.1)).toBeCloseTo(0.5, 12); }); diff --git a/supabase/functions/_shared/ranking/lesson-rank.ts b/supabase/functions/_shared/ranking/lesson-rank.ts index 9c976acd8..cb33ae3f9 100644 --- a/supabase/functions/_shared/ranking/lesson-rank.ts +++ b/supabase/functions/_shared/ranking/lesson-rank.ts @@ -161,14 +161,19 @@ export function normalizeRelevance(value: unknown): number { * NOT an arbitrary tuning knob — it is anchored to a fact about the schema. * `memories.fts` is `to_tsvector('english', key || ' ' || value)` with no * `setweight` call anywhere (migration 00001), so every lexeme in it carries - * Postgres's default D weight, which is `0.1`. A cover containing exactly one - * query lexeme therefore contributes `0.1` to `ts_rank_cd`, and that is the - * weakest thing a real match can be. Putting the saturation point THERE makes - * "matched about as well as a single term does" read as the midpoint of the - * factor, with denser and multi-term covers ranging above it. + * Postgres's default D weight, which is `0.1`. `ts_rank_cd` sums, over the + * covers it finds, each cover's mean lexeme weight divided by `1 +` the + * number of non-query words inside it, so one TIGHT cover — a single query + * lexeme, or query terms sitting side by side — contributes exactly `0.1`. + * That is one cover's ceiling, not a floor on a match: a multi-term cover + * whose terms sit apart scores below it, a negation-only query scores `0` on + * the rows it matches, and several covers add up above it. Putting the + * saturation point THERE makes "matched about as well as a single term + * does" read as the midpoint of the factor, with looser covers below it and + * denser, multi-cover matches above. * * The obvious alternative — saturating at `1` — is monotone and bounded too, - * but it maps the entire plausible `ts_rank_cd` range (roughly 0.1 to 1) into + * but it maps the typical `ts_rank_cd` range (roughly 0.1 to 1) into * `[0.09, 0.5]`, so a graded signal would arrive squashed into the bottom half * of its own scale and lose most of its say in the weighted average. Both * choices are deterministic; this one spends the range on values that occur. From 0a59e5f098fe437dfc3108edcd462180a9d128d1 Mon Sep 17 00:00:00 2001 From: "mads.thines" Date: Sun, 11 Oct 2026 05:49:29 +0000 Subject: [PATCH 7/9] address review comment: docs(ranking): say ts_rank_cd grows with covers, not terms per cover Addresses the pr-reviewer bot's comment: https://github.com/mthines/lorekit/pull/680#discussion_r4240426009 Refs: https://github.com/mthines/lorekit/pull/680 Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> --- packages/mcp-core/src/ranking/lesson-rank.ts | 5 +++-- packages/mcp-core/src/ranking/lexical-rank.spec.ts | 3 ++- supabase/functions/_shared/ranking/lesson-rank.ts | 5 +++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/mcp-core/src/ranking/lesson-rank.ts b/packages/mcp-core/src/ranking/lesson-rank.ts index cb33ae3f9..f8edcdff1 100644 --- a/packages/mcp-core/src/ranking/lesson-rank.ts +++ b/packages/mcp-core/src/ranking/lesson-rank.ts @@ -187,8 +187,9 @@ export const LEXICAL_RANK_SATURATION = 0.1; * Map a raw Postgres `ts_rank_cd` value onto the `[0,1)` relevance factor * `scoreLesson` consumes. * - * `ts_rank_cd` is UNBOUNDED above — it grows with how many query terms a cover - * holds and how tightly they sit — so it cannot be handed to the scorer as-is + * `ts_rank_cd` is UNBOUNDED above — each cover adds at most `0.1` here (less + * the further apart its terms sit), and the total grows with the number of + * covers — so it cannot be handed to the scorer as-is * without letting one dense match swamp every other factor. `r / (r + s)` is * the saturating map: strictly increasing in `r` (so it never reorders what * Postgres ranked), `0` at `0`, asymptotic to `1`, and with no discontinuity to diff --git a/packages/mcp-core/src/ranking/lexical-rank.spec.ts b/packages/mcp-core/src/ranking/lexical-rank.spec.ts index b6b691219..4284f0b5f 100644 --- a/packages/mcp-core/src/ranking/lexical-rank.spec.ts +++ b/packages/mcp-core/src/ranking/lexical-rank.spec.ts @@ -98,7 +98,8 @@ describe('graded relevance changes the ordering the endpoint produces', () => { /** Two rows identical in every factor but how well they matched. */ const twoTermVsOne = [ - // `ts_rank_cd` for a cover holding two query lexemes vs one. + // `ts_rank_cd` for a row with two tight covers (both terms of an OR query + // matched) vs one: each cover adds 0.1, so 0.2 vs 0.1. { scope: 'repo::a/b', key: 'one-term', value: 'x', seen_count: 3, updated_at: updatedAt, relevance: normalizeLexicalRank(0.1) }, { scope: 'repo::a/b', key: 'two-terms', value: 'y', seen_count: 3, updated_at: updatedAt, relevance: normalizeLexicalRank(0.2) }, ]; diff --git a/supabase/functions/_shared/ranking/lesson-rank.ts b/supabase/functions/_shared/ranking/lesson-rank.ts index cb33ae3f9..f8edcdff1 100644 --- a/supabase/functions/_shared/ranking/lesson-rank.ts +++ b/supabase/functions/_shared/ranking/lesson-rank.ts @@ -187,8 +187,9 @@ export const LEXICAL_RANK_SATURATION = 0.1; * Map a raw Postgres `ts_rank_cd` value onto the `[0,1)` relevance factor * `scoreLesson` consumes. * - * `ts_rank_cd` is UNBOUNDED above — it grows with how many query terms a cover - * holds and how tightly they sit — so it cannot be handed to the scorer as-is + * `ts_rank_cd` is UNBOUNDED above — each cover adds at most `0.1` here (less + * the further apart its terms sit), and the total grows with the number of + * covers — so it cannot be handed to the scorer as-is * without letting one dense match swamp every other factor. `r / (r + s)` is * the saturating map: strictly increasing in `r` (so it never reorders what * Postgres ranked), `0` at `0`, asymptotic to `1`, and with no discontinuity to From 51bc31d60df02bc1254fc0ea83d48e5f3924d141 Mon Sep 17 00:00:00 2001 From: "mads.thines" Date: Sun, 11 Oct 2026 06:10:19 +0000 Subject: [PATCH 8/9] address review comment: docs(relevant): say relevance 0 also covers a negation-only q Addresses the pr-reviewer bot's comment: https://github.com/mthines/lorekit/pull/680#discussion_r4240471763 Refs: https://github.com/mthines/lorekit/pull/680 Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> --- packages/schemas/src/shared/relevant.ts | 3 ++- supabase/functions/_shared/schemas/relevant.ts | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/schemas/src/shared/relevant.ts b/packages/schemas/src/shared/relevant.ts index 1c5a29a08..e7855e6ff 100644 --- a/packages/schemas/src/shared/relevant.ts +++ b/packages/schemas/src/shared/relevant.ts @@ -94,7 +94,8 @@ export const RelevantEntrySchema = z.object({ /** * Graded lexical relevance in `[0,1)` — Postgres `ts_rank_cd` over the * `fts` column, saturated into the range (migration 00112). `0` means the - * request carried no `q` at all, never "matched, but badly": a row that did + * request carried no `q`, or a `q` of negated terms only (e.g. + * `-deprecated`), which matches rows without scoring them. A row that did * not match is not returned. It is a LEXICAL score — term overlap and * proximity, deterministically computed — and never a semantic or * embedding similarity. diff --git a/supabase/functions/_shared/schemas/relevant.ts b/supabase/functions/_shared/schemas/relevant.ts index 93121dd7d..970a9b736 100644 --- a/supabase/functions/_shared/schemas/relevant.ts +++ b/supabase/functions/_shared/schemas/relevant.ts @@ -99,7 +99,8 @@ export const RelevantEntrySchema = z.object({ /** * Graded lexical relevance in `[0,1)` — Postgres `ts_rank_cd` over the * `fts` column, saturated into the range (migration 00112). `0` means the - * request carried no `q` at all, never "matched, but badly": a row that did + * request carried no `q`, or a `q` of negated terms only (e.g. + * `-deprecated`), which matches rows without scoring them. A row that did * not match is not returned. It is a LEXICAL score — term overlap and * proximity, deterministically computed — and never a semantic or * embedding similarity. From 49dd5581a34aa8ba3d23aed2e39f1cd6580026af Mon Sep 17 00:00:00 2001 From: "mads.thines" Date: Sun, 11 Oct 2026 06:10:43 +0000 Subject: [PATCH 9/9] address review comment: test(relevant): index AC-11 in the 112 header Addresses the pr-reviewer bot's comment: https://github.com/mthines/lorekit/pull/680#discussion_r4240471768 Refs: https://github.com/mthines/lorekit/pull/680 Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> --- supabase/tests/migrations.test.sql | 2 ++ 1 file changed, 2 insertions(+) diff --git a/supabase/tests/migrations.test.sql b/supabase/tests/migrations.test.sql index 723c4b274..fc5068e29 100644 --- a/supabase/tests/migrations.test.sql +++ b/supabase/tests/migrations.test.sql @@ -10322,6 +10322,8 @@ $$; -- drops org rows the OWNER can otherwise see, `selected` admits the named -- org, and a scope allowlist narrows a call that names no scope. -- AC-10: determinism — two identical calls return the identical order. +-- AC-11: off service_role p_user_id is ignored — auth.uid() is the actor, so a +-- spoofed id reaches neither another user's rows nor loses the caller's own. -- ═════════════════════════════════════════════════════════════════════════ insert into auth.users (id) values