Repository navigation
feat(relevant): grade lexical relevance with ts_rank_cd and cut the candidate window by rank - #680
Conversation
…andidate window by rank `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.
|
The dashboard preview for this PR — redeployed on each push that changes the web app.
Preview always points at this PR's latest commit; Deployment is the immutable build of |
✅ No issues foundRe-review at 49dd558: both doc fixes land in both schema mirrors and the 112 header now indexes AC-11. No new findings; tenancy, the rank-cut window and CI are unchanged. PR body still says 10 ACs (cosmetic). Checked: 14 of 14 changed files read · 3 of 6 dependent files traced · 6 possible issues → 0 confirmed → 0 posted Progress: open review threads 4 → 1 → 1 → 2 → 0 across the last 5 reviews What this change reaches — 8 changed exports · 6 dependent files · 3 checked · 3 not checkedflowchart LR
subgraph pr["Changed in this PR"]
s1["normalizeRelevance<br/>body changed"]:::changed
s2["RelevantQuerySchema<br/>body changed"]:::changed
s3["handleRelevant<br/>body changed"]:::changed
s4["normalizeRelevance<br/>body changed"]:::changed
s5["RelevantQuerySchema<br/>body changed"]:::changed
s6["LEXICAL_RANK_SATURATION<br/>added"]:::changed
smore["+2 more changed exports<br/>listed below"]:::changed
end
s1c1["packages/mcp-core/src/ranking/ · 2 files<br/>? not checked"]:::unknown
s2c1["supabase/functions/memories/handlers/relevant.ts<br/>✓ checked"]:::ok
s2c2["supabase/functions/_shared/schemas/openapi/spec.ts<br/>? not checked"]:::unknown
s3c1["supabase/functions/memories/index.ts<br/>✓ checked"]:::ok
s4c1["supabase/functions/memories/handlers/relevant.ts<br/>✓ checked"]:::ok
s5c1["packages/schemas/src/openapi/spec.ts<br/>✓ checked"]:::ok
s6c1["packages/mcp-core/src/ranking/lexical-rank.spec.ts<br/>✓ checked"]:::ok
s1 -.-> s1c1
s2 --> s2c1
s2 -.-> s2c2
s3 --> s3c1
s4 --> s4c1
s5 --> s5c1
s6 --> s6c1
classDef changed fill:#eef2ff,stroke:#6366f1,color:#1e1b4b
classDef ok fill:#e7f6ec,stroke:#16a34a,color:#14532d
classDef partial fill:#fef9c3,stroke:#ca8a04,color:#713f12
classDef unknown fill:#f3f4f6,stroke:#9ca3af,stroke-dasharray:4 3,color:#374151
classDef bad fill:#fde8e8,stroke:#dc2626,color:#7f1d1d
classDef warn fill:#fff4e5,stroke:#d97706,color:#78350f
classDef imports fill:#f8fafc,stroke:#64748b,stroke-dasharray:4 3,color:#1e293b
Review details
Found Quality — produced 6 → posted inline 0 · cleared 0 · carried forward 0 · deferred 0 · below-bar 0 Run full · 8 lines in delta · tier deep · depth checkout
Nothing to report — integrations (not activated), severity, 0 files skipped.
|
…h its factors Addresses the pr-reviewer bot's comment: #680 (comment) Refs: #680 Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
…T_SELECT projection Addresses the pr-reviewer bot's comment: #680 (comment) Refs: #680 Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
…ler's p_user_id is ignored Addresses the pr-reviewer bot's comment: #680 (comment) Refs: #680 Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
…ause the guard contradicts Addresses the pr-reviewer bot's comment: #680 (comment) Refs: #680 Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
…iling, not a match's floor Addresses the pr-reviewer bot's comment: #680 (comment) Refs: #680 Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
…rs, not terms per cover Addresses the pr-reviewer bot's comment: #680 (comment) Refs: #680 Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
… negation-only q Addresses the pr-reviewer bot's comment: #680 (comment) Refs: #680 Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
Addresses the pr-reviewer bot's comment: #680 (comment) Refs: #680 Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
…pped 00112 #680 merged first with 00112_relevant_ranking.sql, so this PR's migration, its migrations.test.sql section and every reference to them move to 00113 / section 113, as the review thread on the migration agreed. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
…st chosen (#686) * feat(db): reverse-direction activity filters and an opened_count sort for the memory readers 00112 widens lorekit_match_retention with opened_within_days and three min_* floors, appends them to the list/facets/activity/pivot RPCs, and lets lorekit_memory_list sort by opened_count with an integer keyset cursor. migrations.test.sql gains section 112; section 108 now derives the helper signature it checks grants against. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> * feat(api): accept reverse-direction activity filters and sort=opened_count on the memory reads The list/facets/activity/pivot schemas gain opened_within_days and min_seen/read/opened_count (read-only; GroomConditions is unchanged), the edge retention mapping forwards them, and the list paginates the new opened_count sort through a separate integer cursor codec mirrored from mcp-core so paginate.ts and its MCP byte-mirror stay untouched. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> * feat(web): read age & activity filters both ways and sort the Explorer by most chosen Recurrence, Delivered, Chosen and Last agent open gain an At most/At least (More than/Within) toggle on their value list; presets and typed values apply in the selected direction and a row holds one direction at a time. The reverse directions are Explorer-only, so Create retention policy is hidden while one is set. A Sort control (Newest / Most chosen) backs ?sort= and threads opened_count to the list route. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> * docs: record the reverse-direction filters, the opened_count sort and their decision Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> * fix(explorer): address review — icon-sized mobile sort, a recurrence floor that narrows, menu chrome budget - SortSelect is an icon-sized square on mobile (transparent native select over the icon) so the search box keeps its room. - min_seen_count starts at 2: seen_count >= 1 is a table constraint, so a floor of 1 matched every memory. - FilterMenu budgets the direction toggle / Status row in its chrome height. - Pivot's OpenAPI description names the reverse directions; the grooming doc no longer prints a URL spelling the JSON-encoded ?sort= state does not use. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> * fix(cli): register the Explorer's ?sort= param in LORE_PARAM_DEFAULTS deeplink.test.mjs pins LORE_PARAM_DEFAULTS to the Explorer's useUrlState params, so the new sort param needs its default (null = Newest) and a slot in PARAM_ORDER. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> * refactor(explorer): list both directions as rows instead of a toggle, and accept >N / <N The At most/At least toggle was a mode that hid half the values. Each two-way row's value list now shows both directions' presets under two headings, a bare number offers both directions, and >N / <N (also >=, <=, ≥, ≤) pick one side. A strict comparison on a count is stored as the next whole number (>40 -> 41 times or more) so the row reads as the filter it applies; days are not shifted. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> * address review comment: drop the stale "behind a toggle" wording from a story comment Addresses @dash0-dev's comment: #686 (comment) Refs: #686 Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> * address review comment: name the field when a filter's direction switches Addresses the pr-reviewer comment: #686 (comment) Refs: #686 Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> * chore(db): renumber the Explorer migration to 00113 now that #680 shipped 00112 #680 merged first with 00112_relevant_ranking.sql, so this PR's migration, its migrations.test.sql section and every reference to them move to 00113 / section 113, as the review thread on the migration agreed. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> --------- Co-authored-by: mads.thines <[email protected]> Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
Summary
GET /memories/relevantscored relevance as a binary 1-or-nothing, and cut its candidate window by recency. Both were the same limitation:ts_rank_cdis not projectable through PostgREST's query grammar, so from the PostgREST side there was no graded value to sort by and no rank to cut by. The consequence was that ordering among matches was decided entirely by recency and salience, and that on a store with more thanCANDIDATE_LIMITmatches the best-matching lesson could fall outside the window and never be scored at all.Migration
00112moves the candidate fetch intolorekit_memory_relevant_candidates, which returnsts_rank_cd(fts, websearch_to_tsquery('english', q))alongside each row and — when a query is present — orders the window by it. Retrieval stays deterministic and embedding-free: a pure Postgres function over the existingftscolumn, no model, no vector index, no external call on the request path.This is PR 1 of a four-PR stack. PR 2 (RRF rank fusion) depends on it: binary relevance produced a degenerate rank list, and this is what gives RRF a real signal to fuse.
Changes
supabase/migrations/00112_relevant_ranking.sql— newlorekit_memory_relevant_candidates(p_user_id, p_q, p_scopes, p_key_scopes, p_key_org_access, p_key_org_ids, p_limit). Returns the route's narrow projection (scope, key, value, seen_count, updated_at, tags, origin_pr) plusrelevance double precision.STABLE/SECURITY DEFINERwith a pinnedsearch_path, followinglorekit_memory_read_ranking(00085). No new index — theqpath is served bymemories_fts_idx(00001), the no-qpath bymemories_user_updated_at_id_idx(00096).relevant.ts— swaps the PostgREST query for the RPC, maps the raw rank throughnormalizeLexicalRank, and drops the now-stale binary-relevance comment.getMemberOrgIds+applyRestTenantScopego away: the RPC composes the same three SQL functions those helpers mirror, which also removes a round trip (membership becomes a join inside the one query).RELEVANT_SELECTis deleted from both schema mirrors: the RPC'sreturns tableis now the route's only column list.lesson-rank.ts(+ byte mirror) — addsnormalizeLexicalRankandLEXICAL_RANK_SATURATION. Purely additive;scoreLesson,rankLessonsandselectDiverseare untouched, solesson-rank-parity.spec.tsis unaffected.lexical-rank.spec.ts— pins the mapping by value, not merely by property.tenant-scope-usage.spec.ts—relevantmoves fromREST_READ_HANDLERStoRPC_BACKED_ROW_READS(the same movelistmade at 00067), plus a newkey-tenancy usage guardfor org-widening RPC reads.migrations.test.sql§112 — 11 acceptance criteria (AC-11: anauthenticatedcaller'sp_user_idis ignored) plus a function-shape block.decisions.mdentry +CLAUDE.mdindex line,memories/CLAUDE.md, therelevantschema docblocks,llms/template.md→ regeneratedllms.txt.Deliberate deviations from the plan
Three, each with a reason:
SECURITY DEFINER, notSECURITY INVOKER. The api_key tier calls this on the service-role client, where an invoker function applies no RLS and therefore no predicate at all. Definer gives one predicate serving both auth tiers, and it is the shape every sibling analytics RPC already uses.p_key_org_access/p_key_org_idsas well asp_key_scopes.applyRestTenantScopeapplies two narrowings — the scope allowlist and the key's org tenancy (effectiveOrgIds/ownRowsFragment). Moving the query into an RPC without the second would have let apersonal-tenancy key read every org row its owner can see. The RPC composeslorekit_api_token_org_allowed, the SQL twin those helpers mirror, under the repo's existingp_key_org_*parameter names.0.1, not1. The plan suggestedr / (r + 1).memories.ftsapplies nosetweight, so every lexeme carries Postgres's default D weight of0.1and a single-term cover is ats_rank_cdof0.1— meaningr / (r + 1)would squash the entire plausible range into[0.09, 0.5]and a "graded" signal would arrive with most of its say already spent.LEXICAL_RANK_SATURATION = 0.1puts a single-term match at the factor's midpoint. Both choices are deterministic and strictly increasing; this one spends the range on values that occur. The constant is pinned inlexical-rank.spec.tsand its anchor is pinned inmigrations.test.sql§112 AC-4, so if the 0.1 claim is wrong, CI says so rather than the constant drifting quietly.Behaviour change callers should know about
min_scorenow gates across the whole range. Under binary relevance no matched score could drop below(1 + 0.5) / 4 = 0.375— relevance at1, outcome never below its0.5cold-start prior — so anymin_scoreat or under0.375was a no-op. Grading removes that floor, which is the point, but it means a threshold tuned against the old behaviour is now stricter than it was. Documented in the schema docblock,memories/CLAUDE.md, andllms.txt.One thing deliberately not changed: an all-stopword
q(e.g.the) still matches nothing rather than being folded into the no-query path. An emptytsquerymatching nothing is exactly what.textSearchdid; rescuing it would silently answer a different question. Pinned by AC-6.How to verify
All green locally. Lint reports pre-existing warnings only (0 errors).
migrations.test.sqlwas NOT run locally — no Postgres in the sandbox (docs/sandbox.mdtrap 4), and the throwaway PG16 path needs a cluster this box does not have. §112 — including the AC-4 anchor and AC-11 — is proven by CI'sIntegration smokejob instead, which is green at the PR head.