Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
58 changes: 58 additions & 0 deletions docs/decisions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
49 changes: 47 additions & 2 deletions packages/mcp-core/src/auth/tenant-scope-usage.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, like `list`, by the tenancy guard
* after it, because 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');
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
59 changes: 59 additions & 0 deletions packages/mcp-core/src/ranking/lesson-rank.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,65 @@ 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`. `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 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.
*
* 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 — 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
* 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`
Expand Down
138 changes: 138 additions & 0 deletions packages/mcp-core/src/ranking/lexical-rank.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
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 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);
});

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 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) },
];

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);
});
});
Loading
Loading