diff --git a/CLAUDE.md b/CLAUDE.md index 765de1f4f..3ef9391af 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -439,6 +439,7 @@ their rationale inline. **Do not relitigate these.** - **The Explorer's Duplicate Clusters panel is a PANEL, not an instrument** — [rationale](./docs/decisions.md#dashboard-analytics-reads-stay-rest-only) - **Dashboard analytics live on one dedicated `/insights` route** — [rationale](./docs/decisions.md#dashboard-analytics-live-on-one-dedicated-insights-route) - **A COUNT must describe the rows its LIST returns — retention thresholds reach all four readers** — [rationale](./docs/decisions.md#a-count-must-describe-the-rows-its-list-returns) +- **Age/activity conditions read both ways (00113), but the reverse directions are READ-only — never in `GroomConditionsSchema`** — [rationale](./docs/decisions.md#the-ageactivity-conditions-read-both-ways-but-only-one-way-reaches-a-policy) - **A read must never restamp `updated_at`** — [rationale](./docs/decisions.md#a-read-must-never-restamp-updated_at) - **Lore value is the RATIO `opened_count / read_count`, and `/insights` leads with the bill** — [rationale](./docs/decisions.md#lore-value-is-a-ratio-and-insights-leads-with-the-bill) - **A citation is the agent's word, and it is a fact about a RUN** — [rationale](./docs/decisions.md#a-citation-is-the-agents-word-and-it-is-a-fact-about-a-run) diff --git a/docs/decisions.md b/docs/decisions.md index 90e4e35a8..9118d1cb7 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -749,6 +749,30 @@ directories (`packages/cli/skill/*`) — the Claude-plugin-vendored mirror under `scripts/codegen/sync-plugin-skill.mjs --check` (run inside the `plugin` CI job), so the two guards compose instead of duplicating each other's job. +## The age/activity conditions read both ways, but only one way reaches a policy + +The five retention conditions all point the archive way ("N or fewer", "not opened for N days"), so the +Explorer could show the dead weight and never the load-bearing lore. 00113 adds the mirror of four of them +— `opened_within_days`, `min_seen_count`, `min_read_count`, `min_opened_count` — plus `sort=opened_count`. + +They are **read conditions only**: they reach the list and the three aggregates (the rule below), and are +deliberately absent from `GroomConditionsSchema`. Archiving "everything chosen at least 10 times" is not a +policy anyone wants, and a schema able to express it is one careless edit from archiving the most useful +lore in the store. The Explorer therefore hides **Create retention policy** while one is set, rather than +saving a policy that would archive a different set from the one on screen +(`retentionConditionsToGroomConditions` drops them as a second line). + +In the menu each pair shares ONE row and label, and that row's value list holds BOTH directions as ordinary +rows under two headings; a row holds one direction at a time (setting one clears the other). Two menu rows +per metadata row would double the level-one list for a choice that is a property of the row. A direction +TOGGLE above the value list was tried first and replaced: it was a mode that hid half the values, cost a +click and a Tab stop before they were even visible, and typing could only ever fill the side it was set to. +As rows, typing follows the list: a bare number offers both directions, `>N` / `40` → 41) so the row reads as the filter it +applies. `opened_within_days` does **not** inherit 00100's never-opened fallback: a lesson +nothing ever opened was not "opened recently". Created has no reverse — the date-range picker already +answers "created within". + ## A COUNT must describe the rows its LIST returns The five retention conditions (`min_age_days`, `unseen_days`, `max_seen_count`, `max_read_count`, diff --git a/docs/key-files.md b/docs/key-files.md index 3fa16cc05..43664fa57 100644 --- a/docs/key-files.md +++ b/docs/key-files.md @@ -177,6 +177,8 @@ else. Grouped loosely by subsystem in migration/PR order. | `packages/mcp-core/src/scope/memory-ref.spec.ts` | One case table run against THREE implementations — mcp-core's copy, the edge copy (imported directly from `supabase/functions/_shared/`), and the CLI's `resolveScopeArg` — because the grammar has to mean one thing on every surface. It caught the `validateScope` delegation on its first run: the edge copy split the branch-scope case one `::` early | | `packages/mcp-core/src/memory/read-refs.ts` (+ its edge mirror) | The pure core behind batch `memory.read` (`refs`), shared by MCP's `toolRead` and REST's `handleRead`. Import-free by construction — it takes only already-PARSED `{scope,key}` pairs, never re-deriving the `scope::key` grammar `parseMemoryRefs` alone owns. `groupRefsByScope` collapses N refs into one `{scope, keys}` group per DISTINCT scope (plan D5) — the transport issues exactly one `.eq('scope', s).in('key', keys)` query per group, awaited concurrently, instead of one query per ref. `isQueryableKey` rejects a key carrying a comma/paren/quote/backslash BEFORE it reaches the query, since postgrest-js's `.in()` filter quotes a comma/paren but does not escape a quote or backslash inside the value — a bad ref becomes a `missing` entry rather than a malformed filter list. `missingRefs` reports every unresolved ref as its own caller-supplied `scope::key` string, in REQUEST order — but it only ever sees what `parseMemoryRefs` RETURNED, so a ref dropped as malformed or truncated past the 32-ref cap reaches neither `entries` nor `missing` (the gap is visible as `lorekit.refs.requested` vs `lorekit.refs.count`); matching stays against the VERBATIM stored scope, deliberately diverging from the singular `memory.read` path's `validateScope` normalisation, because the transports differ in what they hand `memory_write` and no reader can infer how a row was cased — see `docs/decisions.md#memoryread-batching-is-conditional-verbatim-scoped-and-inflates-a-signal-it-doesnt-count` | | `supabase/migrations/00108_retention_conditions_on_aggregates.sql` | Gives `lorekit_memory_facets` (24→31 args), `lorekit_memory_activity` (26→31) and `lorekit_memory_pivot` (27→34) the five retention thresholds `lorekit_memory_list` has had since 00092, plus a `created_since`/`created_until` pair on facets and pivot (activity's own `since`/`until` already bound `created_at`). **The bug it fixes is a DISAGREEMENT, not a missing feature**: only the list applied the thresholds, so `max_opened_count => 0` narrowed the Explorer's rows and left its facet counts, stat cards and matrix counting the un-narrowed population — every number on the page describing a different set from the one below it. Introduces `lorekit_match_retention`, ONE inlinable row test now composed by all four readers, so a fourth inline copy cannot drift (00101 records that exact drift happening once already between two copies). It takes CUTOFF INSTANTS rather than day counts, which is what keeps it `immutable` and index-usable — each caller resolves its `now()`-relative bounds once per query in a declare block — and it holds 00100's never-opened rule (`coalesce(last_opened_at, created_at)`) in one place. **Applied in the `base` CTE's WHERE, never as an `ok_*` flag**, so a threshold narrows even the SELF-EXCLUDED dimension: self-exclusion exists so the dimension you are standing in still lists what you could switch to, and switching `host` does not stop you looking at lore older than 30 days. The three aggregates are DROPPED at their exact old signatures and recreated (`create or replace` cannot change a signature, and leaving both overloads makes every PostgREST named-arg call ambiguous — 00092's footgun); `lorekit_memory_list` is a true `create or replace` at its verbatim 00105 signature, so its grants persist. Pinned by §108 | +| `supabase/migrations/00113_explorer_reverse_activity_filters_and_opened_sort.sql` | The Explorer's REVERSE directions: `opened_within_days` and the `min_seen_count`/`min_read_count`/`min_opened_count` floors, the mirror of four of the five retention thresholds, for "which lore IS used?" rather than "what can I archive?". Widens `lorekit_match_retention` 10→14 args (the 10-arg form is dropped LAST, after every caller is recreated), appends the four to `_facets`/`_activity`/`_pivot` and, with a `p_cursor_count`, to `lorekit_memory_list`, which also accepts `p_sort => 'opened_count'` (the format() splices `$44` or `$29` as the keyset placeholder). `opened_within_days` does NOT age a never-opened lesson from `created_at` — the opposite of 00100's rule for `unseen_days`, on purpose. READ-only: no groom RPC or `GroomConditionsSchema` change. Restores the `anon` revoke on `lorekit_memory_list` that 00105's drop-and-recreate lost. Pinned by §113; §108's helper-ACL check now derives the signature | +| `packages/mcp-core/src/rest/count-cursor.ts` | The `sort=opened_count` keyset cursor (`{ id, opened_count }`, base64url) and its `buildCountPage`. Separate from `_shared/api/paginate.ts` because that cursor carries a timestamp and is byte-mirrored into the MCP function, which has no count sort. Mirrored verbatim into `supabase/functions/_shared/api/count-cursor.ts`, guarded by `edge-parity.spec.ts`; tested by `count-cursor.spec.ts` | | `supabase/functions/_shared/api/retention.ts` | 00108's fix one layer up: `retentionFrom` picks the five thresholds off a validated query or body, `retentionRpcParams` maps them to the `p_*` arguments all four RPCs share. Used by `list.ts` and the three aggregate handlers, so a route cannot be given the parameter and then forget to forward it. `?? null` and never a truthiness check — `max_opened_count => 0` is the whole point of 00105, so a falsy test turns the most useful threshold in the set into no filter at all | | `packages/web/src/lib/retention-filter.ts` → `retentionConditionsToAggregateBody` | The client half. Delegates to `retentionConditionsToListBody` (the field names are identical by construction — one shared `retentionConditionBodyFields` spread in the schemas) but is typed against `ListFacetsBody`, so a schema that stops carrying one of the five is a TYPE error rather than a threshold the aggregate silently ignores. Its spec reads each field back off the PARSED schema output rather than asserting `safeParse(...).success`: these schemas are not `.strict()`, so zod strips an unknown key and still reports success — a success-only check would pass against the pre-00108 routes and prove nothing | | `packages/cli/src/core/lessons.mjs` → `citationHint` | The Stop-hook ask. Reads the session's already-recorded shown-set and names those `scope::key` refs, capped at `CITED_HINT_MAX` (8) with a `(+N more)` tail — an agent picking from a list is a different task from one recalling a convention. Empty when nothing was injected (with no candidates the ask is an instruction to invent references). Refs are taken in INJECTION order, so the SessionStart set leads | diff --git a/docs/otel.md b/docs/otel.md index 44f5dd04b..45ae34b35 100644 --- a/docs/otel.md +++ b/docs/otel.md @@ -917,6 +917,7 @@ attribute here. | `lore.memory_archive_toggled` | A memory was archived or restored | `lorekit.lore.memory.archive_action`, `.archive_outcome` | | `lore.filter_changed` | Any filter-bar mutation, age/activity thresholds included | `lorekit.lore.filter.action`, `.field` (optional), `.operator` (optional), `.count`, `.retention_count` | | `lore.status_changed` | Active / Archived / Expiring | `lorekit.lore.status` | +| `lore.sort_changed` | The list order was picked (Newest / Most chosen) | `lorekit.lore.sort` (`created_at`/`opened_count`) | | `lore.search_committed` | The debounced search settled | `lorekit.lore.search.length` | | `lore.scope_selected` | A scope chip or Browse-all row was picked | `lorekit.lore.scope.type`, `.source` (`strip`/`browse-all`) | | `lore.range_changed` | Any of the FIVE controls that write `?range=` | `lorekit.lore.range.preset`, `.source`, `.span_days` (omitted when unbounded) | diff --git a/packages/cli/src/shared/deeplink-pure.mjs b/packages/cli/src/shared/deeplink-pure.mjs index 5e07a4c83..786a84edc 100644 --- a/packages/cli/src/shared/deeplink-pure.mjs +++ b/packages/cli/src/shared/deeplink-pure.mjs @@ -46,9 +46,13 @@ export const LORE_PARAM_DEFAULTS = { // "unfiltered". filters: null, tags: [], // string[] — legacy label filter (AND across labels); [] means "no filter". Still READ by the app, superseded by `filters` - // RetentionConditions | null — the retention-preview trio (min age / unseen-for - // / seen-at-most). `null` means no narrowing; absent from the URL when unset. + // RetentionConditions | null — the Explorer's age/activity conditions (the + // five retention thresholds plus, since 00113, their four reverse directions). + // `null` means no narrowing; absent from the URL when unset. retention: null, + // 'created_at' | 'opened_count' | null — the list order (00113). `null` is + // Newest, the default, so an unsorted link carries no `sort` param. + sort: null, // 'active' | 'archived' | 'expiring' | null — the Explorer's Status control. // `null`, NOT 'active', is the default for `filters`' reason: the app has to // tell "absent" from an explicit choice, because an absent `status` falls back @@ -62,9 +66,9 @@ export const LORE_PARAM_DEFAULTS = { // A stable, readable param order (also makes URLs deterministic for tests). // Mirrors the `useUrlState` call order in `LoreExplorer.tsx` (+ the `lesson` -// param last), so `filters` and `tags` sit between `owner` and `status`, and -// `retention` sits between `tags` and `status`. `scope` precedes `lesson` so a -// lesson link reads `?scope=…&lesson=…`. +// param last), so `filters` and `tags` sit between `owner` and `status`, +// `retention` sits between `tags` and `status`, and `sort` follows `archived`. +// `scope` precedes `lesson` so a lesson link reads `?scope=…&lesson=…`. const PARAM_ORDER = [ 'scope', 'q', @@ -75,6 +79,7 @@ const PARAM_ORDER = [ 'retention', 'status', 'archived', + 'sort', 'lesson', ]; diff --git a/packages/cli/src/shared/mirror-pairs.mjs b/packages/cli/src/shared/mirror-pairs.mjs index a70b1da66..2a15599cf 100644 --- a/packages/cli/src/shared/mirror-pairs.mjs +++ b/packages/cli/src/shared/mirror-pairs.mjs @@ -74,6 +74,7 @@ export const mirrorPairs = [ { core: 'packages/mcp-core/src/telemetry/usage-stats.ts', edge: 'supabase/functions/_shared/telemetry/usage-stats.ts', driftChecked: true }, { core: 'packages/mcp-core/src/limits/expiring-window.ts', edge: 'supabase/functions/_shared/limits/expiring-window.ts', driftChecked: true }, { core: 'packages/mcp-core/src/rest/cors-origins.ts', edge: 'supabase/functions/_shared/api/cors-origins.ts', driftChecked: true }, + { core: 'packages/mcp-core/src/rest/count-cursor.ts', edge: 'supabase/functions/_shared/api/count-cursor.ts', driftChecked: true }, { core: 'packages/mcp-core/src/scope/scope-type-attribute.ts', edge: 'supabase/functions/_shared/scope/scope-type-attribute.ts', driftChecked: true }, // Which row an UNSCOPED read resolves to. Import-free on both sides (it only // parses the scope prefix and compares strings), so the byte comparison diff --git a/packages/mcp-core/src/rest/count-cursor.spec.ts b/packages/mcp-core/src/rest/count-cursor.spec.ts new file mode 100644 index 000000000..9ff6cd966 --- /dev/null +++ b/packages/mcp-core/src/rest/count-cursor.spec.ts @@ -0,0 +1,67 @@ +import { describe, it, expect } from 'vitest'; +import { buildCountPage, decodeCountCursor, encodeCountCursor } from './count-cursor.ts'; + +const ID = '0f8fad5b-d9cb-469f-a165-70867728950e'; +const ID2 = '7c9e6679-7425-40de-944b-e07fc1f90ae7'; + +/** A `paginate.ts`-shaped date cursor, built the way that module builds one. */ +function dateCursor(id: string, ts: string): string { + return btoa(JSON.stringify({ id, updated_at: ts })).replace(/\+/g, '-').replace(/\//g, '_').replace(/=/g, ''); +} + +describe('count cursor codec', () => { + it('round-trips an id and a count', () => { + expect(decodeCountCursor(encodeCountCursor(ID, 12))).toEqual({ id: ID, count: 12 }); + }); + + it('keeps a count of 0 — the never-chosen tail is a real page boundary', () => { + expect(decodeCountCursor(encodeCountCursor(ID, 0))).toEqual({ id: ID, count: 0 }); + }); + + it('emits URL-safe text with no padding', () => { + expect(encodeCountCursor(ID, 123_456)).toMatch(/^[A-Za-z0-9_-]+$/); + }); + + it('refuses a DATE cursor, so flipping sort mid-pagination restarts at page one', () => { + expect(decodeCountCursor(dateCursor(ID, '2026-10-01T00:00:00Z'))).toBeNull(); + }); + + it.each([ + ['not base64 at all', '%%%'], + ['valid base64, not JSON', btoa('hello')], + ['JSON, not an object', btoa('42')], + ['non-uuid id', btoa(JSON.stringify({ id: 'abc', opened_count: 1 }))], + ['string count', btoa(JSON.stringify({ id: ID, opened_count: '1' }))], + ['fractional count', btoa(JSON.stringify({ id: ID, opened_count: 1.5 }))], + ['negative count', btoa(JSON.stringify({ id: ID, opened_count: -1 }))], + ['count past int4', btoa(JSON.stringify({ id: ID, opened_count: 2_147_483_648 }))], + ])('fails closed on %s', (_label, raw) => { + expect(decodeCountCursor(raw)).toBeNull(); + }); +}); + +describe('buildCountPage', () => { + const rows = [ + { id: ID, opened_count: 9 }, + { id: ID2, opened_count: 3 }, + { id: '00000000-0000-4000-8000-000000000001', opened_count: 3 }, + ]; + + it('reports more pages from the overflow row and cursors on the LAST returned row', () => { + const page = buildCountPage(rows, 2); + expect(page.entries).toHaveLength(2); + expect(page.hasMore).toBe(true); + expect(decodeCountCursor(page.nextCursor ?? '')).toEqual({ id: ID2, count: 3 }); + }); + + it('has no cursor on the last page', () => { + const page = buildCountPage(rows, 5); + expect(page.hasMore).toBe(false); + expect(page.nextCursor).toBeNull(); + }); + + it('reads a missing counter as 0 rather than minting an undecodable cursor', () => { + const page = buildCountPage([{ id: ID }, { id: ID2 }], 1); + expect(decodeCountCursor(page.nextCursor ?? '')).toEqual({ id: ID, count: 0 }); + }); +}); diff --git a/packages/mcp-core/src/rest/count-cursor.ts b/packages/mcp-core/src/rest/count-cursor.ts new file mode 100644 index 000000000..8c314b4d0 --- /dev/null +++ b/packages/mcp-core/src/rest/count-cursor.ts @@ -0,0 +1,78 @@ +/** + * The keyset cursor for the `sort=opened_count` list — "most chosen first" + * (migration 00113). + * + * Pure and shared (mirrored into `supabase/functions/_shared/api/count-cursor.ts`, + * guarded by `edge-parity.spec.ts`) so the codec is unit-tested here and the + * deployed copy cannot drift from what was tested. + * + * Why it is not one more case in `_shared/api/paginate.ts`: that cursor carries + * a TIMESTAMP — both date sorts put their last row's value in it and the list + * RPC compares it as `timestamptz` — and `paginate.ts` is byte-mirrored into the + * MCP function, which has no count sort. An integer bound needs an integer + * field, so this is its own codec with its own shape: `{ id, opened_count }`. + * + * The two formats cannot be mistaken for each other. `paginate.ts`'s decoder + * finds neither `updated_at` nor `created_at` in a count cursor and drops it, + * and this decoder finds no integer `opened_count` in a date cursor and drops + * that. A caller who flips `sort` mid-pagination therefore restarts at page one + * rather than receiving a page cut at the wrong place. + */ + +/** The decoded keyset position: the last row's id and its `opened_count`. */ +export interface CountCursor { + id: string; + count: number; +} + +/** A cursor's `id` is a `memories` primary key, which is a `uuid`. */ +const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +/** `opened_count` is a Postgres `integer`, so a bound past int4 cannot be a real row's value. */ +const INT4_MAX = 2_147_483_647; + +/** base64url without padding — the same alphabet `paginate.ts` uses. */ +export function encodeCountCursor(id: string, count: number): string { + return btoa(JSON.stringify({ id, opened_count: count })) + .replace(/\+/g, '-') + .replace(/\//g, '_') + .replace(/=/g, ''); +} + +/** + * Decode a count cursor, or `null` for anything that is not one. + * + * Fails CLOSED: the values reach the RPC as typed parameters (`uuid`, + * `integer`), so a malformed one would be a 500 rather than an injection — and + * the right answer to a cursor that does not parse is the first page, the same + * contract `paginate.ts` keeps. + */ +export function decodeCountCursor(raw: string): CountCursor | null { + try { + const b64 = raw.replace(/-/g, '+').replace(/_/g, '/'); + const pad = (4 - (b64.length % 4)) % 4; + const parsed: unknown = JSON.parse(atob(b64 + '='.repeat(pad))); + if (!parsed || typeof parsed !== 'object') return null; + const { id, opened_count: count } = parsed as { id?: unknown; opened_count?: unknown }; + if (typeof id !== 'string' || !UUID_RE.test(id)) return null; + if (typeof count !== 'number' || !Number.isInteger(count) || count < 0 || count > INT4_MAX) return null; + return { id, count }; + } catch { + return null; + } +} + +/** + * The page and its next cursor, from `limit + 1` rows — the overflow row is how + * `hasMore` is known, exactly as `paginate.ts`'s `buildPage` reads it. + */ +export function buildCountPage( + rows: T[], + limit: number, +): { entries: T[]; hasMore: boolean; nextCursor: string | null } { + const hasMore = rows.length > limit; + const entries = hasMore ? rows.slice(0, limit) : rows; + const last = entries[entries.length - 1]; + const nextCursor = hasMore && last ? encodeCountCursor(last.id, last.opened_count ?? 0) : null; + return { entries, hasMore, nextCursor }; +} diff --git a/packages/schemas/src/domain/memory-body.spec.ts b/packages/schemas/src/domain/memory-body.spec.ts index 7e7e972b5..c0cfd54a0 100644 --- a/packages/schemas/src/domain/memory-body.spec.ts +++ b/packages/schemas/src/domain/memory-body.spec.ts @@ -22,6 +22,7 @@ import { FILTER_VALUE_MAX_CHARS, ListFacetsBodySchema, ListMemoriesBodySchema, + ListMemoriesQuerySchema, PivotBodySchema, } from './memory.ts'; @@ -40,6 +41,28 @@ describe('ListMemoriesBodySchema', () => { expect(r.host_mode).toBe('in'); }); + it('accepts the opened_count sort and the reverse directions (00113)', () => { + const r = ListMemoriesBodySchema.parse({ + sort: 'opened_count', + min_opened_count: 10, + min_read_count: 100, + min_seen_count: 2, + opened_within_days: 7, + }); + expect(r.sort).toBe('opened_count'); + expect(r.min_opened_count).toBe(10); + expect(r.opened_within_days).toBe(7); + expect(ListMemoriesBodySchema.safeParse({ sort: 'read_count' }).success).toBe(false); + }); + + it('coerces the reverse directions on the query transport like every other threshold', () => { + const r = ListMemoriesQuerySchema.parse({ sort: 'opened_count', min_opened_count: '10', opened_within_days: '7' }); + expect(r.sort).toBe('opened_count'); + expect(r.min_opened_count).toBe(10); + expect(r.opened_within_days).toBe(7); + expect(ListMemoriesQuerySchema.safeParse({ min_opened_count: '0' }).success).toBe(false); + }); + it('accepts a dimension at the documented bound', () => { const r = ListMemoriesBodySchema.safeParse({ host: hosts(FILTER_VALUES_MAX) }); expect(r.success, JSON.stringify(r.error?.issues?.[0])).toBe(true); @@ -139,6 +162,11 @@ describe('retention thresholds on the aggregate body schemas', () => { ['max_seen_count', 1], ['max_read_count', 200], ['max_opened_count', 0], + // 00113 — the reverse directions ride the same spread. + ['opened_within_days', 7], + ['min_seen_count', 2], + ['min_read_count', 100], + ['min_opened_count', 10], ] as const; for (const name of Object.keys(AGGREGATES) as (keyof typeof AGGREGATES)[]) { @@ -166,6 +194,21 @@ describe('retention thresholds on the aggregate body schemas', () => { // only coerced on the query transport. expect(AGGREGATES[name].safeParse({ ...base(name), min_age_days: '7' }).success).toBe(false); }); + + it('starts each reverse direction at its first value that narrows anything', () => { + // A floor of 0 matches every row; "opened within 0 days" matches none + // the caller could mean. Both are 400s rather than silent no-ops. + for (const field of ['opened_within_days', 'min_read_count', 'min_opened_count']) { + expect(AGGREGATES[name].safeParse({ ...base(name), [field]: 0 }).success, field).toBe(false); + expect(AGGREGATES[name].safeParse({ ...base(name), [field]: 1 }).success, field).toBe(true); + } + // `seen_count >= 1` is a table constraint (00059), so "written at least + // once" is every memory — the recurrence floor starts at 2. + expect(AGGREGATES[name].safeParse({ ...base(name), min_seen_count: 1 }).success).toBe(false); + expect(AGGREGATES[name].safeParse({ ...base(name), min_seen_count: 2 }).success).toBe(true); + expect(AGGREGATES[name].safeParse({ ...base(name), opened_within_days: 3651 }).success).toBe(false); + expect(AGGREGATES[name].safeParse({ ...base(name), min_opened_count: 100_001 }).success).toBe(false); + }); }); } diff --git a/packages/schemas/src/domain/memory.ts b/packages/schemas/src/domain/memory.ts index 0e76c8d13..c8429b7e0 100644 --- a/packages/schemas/src/domain/memory.ts +++ b/packages/schemas/src/domain/memory.ts @@ -123,8 +123,15 @@ export const TimestampFilterSchema = z 'must be an ISO date (YYYY-MM-DD) or timestamp', ); -/** Sort column for `GET /memories`. The cursor encodes whichever is chosen. */ -export const MemorySortSchema = z.enum(['updated_at', 'created_at']); +/** + * Sort column for `GET /memories`. The cursor encodes whichever is chosen. + * + * `opened_count` (migration 00113) is "most chosen first": descending on the + * deliberate-fetch counter, `id` breaking ties. It pages through a cursor of + * its own (`count-cursor.ts`), because a keyset bound on an integer column + * cannot ride in the timestamp cursor the two date sorts share. + */ +export const MemorySortSchema = z.enum(['updated_at', 'created_at', 'opened_count']); export type MemorySort = z.infer; /** @@ -186,6 +193,28 @@ const ValueListSchema = z.string().min(1).max(2048); * The bounds are copied rather than imported from `retention.ts`: that file is * its own domain file for the reason its header states, and `memory.spec.ts` * pins the two in step. + * + * ## The reverse directions (migration 00113) are READ-ONLY conditions + * + * The five above all point the retention way — "N or fewer", "not opened for + * N days" — because they are what a groomer asks before archiving. The last + * four are their mirrors, for the opposite question ("what is used the most?"): + * + * - `opened_within_days` — last deliberately fetched within N days. A lesson + * NEVER opened does not match, however new: unlike `unseen_days` it does not + * age a never-opened lesson from `created_at`, because "opened recently" is a + * claim about an agent's behaviour that no agent made. + * - `min_seen_count` / `min_read_count` / `min_opened_count` — at least N. + * Each starts at the first value that can narrow anything: 1 for the two + * read counters (a floor of 0 matches every row), and 2 for `min_seen_count`, + * because `seen_count >= 1` is a table constraint (00059) and "written at + * least once" describes every memory there is. + * + * They are deliberately ABSENT from `GroomConditionsSchema`: archiving + * "everything chosen at least 10 times" is not a policy anyone wants, and a + * policy able to express it is one careless edit from archiving the most useful + * lore in the store. So they ride here, on the four read routes, and nowhere a + * write can reach them. */ const retentionConditionBodyFields = { min_age_days: z.number().int().min(1).max(3650).optional(), @@ -193,6 +222,10 @@ const retentionConditionBodyFields = { max_seen_count: z.number().int().min(0).max(100_000).optional(), max_read_count: z.number().int().min(0).max(100_000).optional(), max_opened_count: z.number().int().min(0).max(100_000).optional(), + opened_within_days: z.number().int().min(1).max(3650).optional(), + min_seen_count: z.number().int().min(2).max(100_000).optional(), + min_read_count: z.number().int().min(1).max(100_000).optional(), + min_opened_count: z.number().int().min(1).max(100_000).optional(), } as const; /** @@ -207,6 +240,10 @@ const retentionConditionQueryFields = { max_seen_count: z.coerce.number().int().min(0).max(100_000).optional(), max_read_count: z.coerce.number().int().min(0).max(100_000).optional(), max_opened_count: z.coerce.number().int().min(0).max(100_000).optional(), + opened_within_days: z.coerce.number().int().min(1).max(3650).optional(), + min_seen_count: z.coerce.number().int().min(2).max(100_000).optional(), + min_read_count: z.coerce.number().int().min(1).max(100_000).optional(), + min_opened_count: z.coerce.number().int().min(1).max(100_000).optional(), } as const; export const ListMemoriesQuerySchema = z.object({ diff --git a/packages/schemas/src/llms/template.md b/packages/schemas/src/llms/template.md index bde66cd50..b926a73ca 100644 --- a/packages/schemas/src/llms/template.md +++ b/packages/schemas/src/llms/template.md @@ -241,6 +241,28 @@ These work on the aggregate reads too — `/memories/facets`, `/memories/pivot` `/memories/activity` all take the same five, so a facet count describes the same set the list returns rather than the un-narrowed population. +### Finding lore that IS used + +Four more thresholds read the same rows the other way. They are READ-only — a retention policy +cannot carry them — and they reach the same four routes: + +| Parameter | Range | Keeps a memory when | +|-----------|-------|---------------------| +| `opened_within_days` | 1–3650 | an agent deliberately opened it within N days (one NEVER opened does not match) | +| `min_seen_count` | 2–100000 | it has been written at least N times (every memory is written once) | +| `min_read_count` | 1–100000 | it has been DELIVERED at least N times | +| `min_opened_count` | 1–100000 | an agent CHOSE to fetch it at least N times | + +`sort=opened_count` on `GET /memories` / `POST /memories/list` orders by that same counter, most +chosen first (ties by id); its `nextCursor` is only valid for that sort. + +```bash +# The lore agents reach for most, top first +curl -H "Authorization: Bearer lk_ro_…" \ + "https://pqokxlhvnosogizsjztg.supabase.co/functions/v1/memories?\ +min_opened_count=5&sort=opened_count" +``` + ### Finding lore that is about to expire `?expiring_within_days=N` (1–365) narrows the list to memories whose TTL runs out soon — those diff --git a/packages/schemas/src/openapi/spec.ts b/packages/schemas/src/openapi/spec.ts index dfa967d8c..2dc04e6a4 100644 --- a/packages/schemas/src/openapi/spec.ts +++ b/packages/schemas/src/openapi/spec.ts @@ -286,8 +286,9 @@ export function generateSpec(baseUrl = 'https://pqokxlhvnosogizsjztg.supabase.co 'An invalid `scope` is a `400`, not a silently ignored filter — the menu counts and the ' + 'list they drill into agree on what a scope is. The `created_since`/`created_until` window ' + 'and the five retention thresholds (`min_age_days`, `unseen_days`, `max_seen_count`, ' + - '`max_read_count`, `max_opened_count`) narrow the counted population exactly as they do on ' + - '`GET /memories`, so a count matches the list under the same parameters. They emit no facet ' + + '`max_read_count`, `max_opened_count`) and their four reverse directions (`opened_within_days`, ' + + '`min_seen_count`, `min_read_count`, `min_opened_count`) narrow the counted population exactly ' + + 'as they do on `GET /memories`, so a count matches the list under the same parameters. They emit no facet ' + 'rows of their own — they are comparisons, not enumerable dimensions. `q`, `key` and ' + '`expiring_within_days` remain unmirrored, so with those active a count is an upper bound.', security, request: { query: ListFacetsQuerySchema }, @@ -321,8 +322,8 @@ export function generateSpec(baseUrl = 'https://pqokxlhvnosogizsjztg.supabase.co '`row in [x] AND col in [y]` and asks again still gets every other cell back — that ' + 'is what keeps a drilled-in grid navigable instead of collapsing to the clicked cell. ' + 'A pair counting zero emits no cell. `truncated` reports whether `limit` cut the grid. ' + - 'The `created_since`/`created_until` window and the five retention thresholds narrow the ' + - 'counted population, matching `GET /memories/facets`.', + 'The `created_since`/`created_until` window, the five retention thresholds and their four ' + + 'reverse directions narrow the counted population, matching `GET /memories/facets`.', security, request: { query: PivotQuerySchema }, responses: { 200: { description: 'Pivot cells', content: { 'application/json': { schema: PivotResponseSchema } } }, @@ -346,7 +347,8 @@ export function generateSpec(baseUrl = 'https://pqokxlhvnosogizsjztg.supabase.co 'An invalid `scope` is a `400`, not a silently ignored filter — the same rule the read ' + 'counterpart `GET /memories/read-activity` follows. The five retention thresholds ' + '(`min_age_days`, `unseen_days`, `max_seen_count`, `max_read_count`, `max_opened_count`) ' + - 'narrow the counted population exactly as they do on `GET /memories`, so a series matches ' + + 'and their four reverse directions (`opened_within_days`, `min_seen_count`, `min_read_count`, ' + + '`min_opened_count`) narrow the counted population exactly as they do on `GET /memories`, so a series matches ' + 'the list under the same parameters. There is no `created_since`/`created_until` pair here, ' + 'unlike `/facets` and `/pivot`: this route’s own `since`/`until` already bound `created_at`.', security, request: { query: ActivityQuerySchema }, diff --git a/packages/web/public/llms.txt b/packages/web/public/llms.txt index b028eb2dc..2fbb18c8a 100644 --- a/packages/web/public/llms.txt +++ b/packages/web/public/llms.txt @@ -768,6 +768,28 @@ These work on the aggregate reads too — `/memories/facets`, `/memories/pivot` `/memories/activity` all take the same five, so a facet count describes the same set the list returns rather than the un-narrowed population. +### Finding lore that IS used + +Four more thresholds read the same rows the other way. They are READ-only — a retention policy +cannot carry them — and they reach the same four routes: + +| Parameter | Range | Keeps a memory when | +|-----------|-------|---------------------| +| `opened_within_days` | 1–3650 | an agent deliberately opened it within N days (one NEVER opened does not match) | +| `min_seen_count` | 2–100000 | it has been written at least N times (every memory is written once) | +| `min_read_count` | 1–100000 | it has been DELIVERED at least N times | +| `min_opened_count` | 1–100000 | an agent CHOSE to fetch it at least N times | + +`sort=opened_count` on `GET /memories` / `POST /memories/list` orders by that same counter, most +chosen first (ties by id); its `nextCursor` is only valid for that sort. + +```bash +# The lore agents reach for most, top first +curl -H "Authorization: Bearer lk_ro_…" \ + "https://pqokxlhvnosogizsjztg.supabase.co/functions/v1/memories?\ +min_opened_count=5&sort=opened_count" +``` + ### Finding lore that is about to expire `?expiring_within_days=N` (1–365) narrows the list to memories whose TTL runs out soon — those diff --git a/packages/web/src/components/lore/FilterMenu.test.stories.tsx b/packages/web/src/components/lore/FilterMenu.test.stories.tsx index eccfbd314..f4291b51a 100644 --- a/packages/web/src/components/lore/FilterMenu.test.stories.tsx +++ b/packages/web/src/components/lore/FilterMenu.test.stories.tsx @@ -16,7 +16,8 @@ import { type FilterField, } from '@/lib/filters'; import { - RETENTION_FIELDS, + RETENTION_MENU_FIELDS, + requireRetentionField, type RetentionConditions, type RetentionField, } from '@/lib/retention-filter'; @@ -752,11 +753,13 @@ export const AgeAndActivityRowsSitBelowTheDimensions: StoryObj { + // Five ROWS, not nine conditions: a reverse direction (00113) lives inside + // its row's value list, never as a row of its own. const rows = canvas.getAllByRole('option'); - await expect(rows).toHaveLength(FILTER_FIELDS.length + RETENTION_FIELDS.length); + await expect(rows).toHaveLength(FILTER_FIELDS.length + RETENTION_MENU_FIELDS.length); const retentionRows = rows.slice(FILTER_FIELDS.length); await expect(retentionRows.map((r) => r.textContent?.trim())).toEqual( - RETENTION_FIELDS.map((d) => d.label), + RETENTION_MENU_FIELDS.map((f) => requireRetentionField(f).label), ); }); @@ -894,6 +897,82 @@ export const TypingAValueOffersItAsACustomThreshold: StoryObjN` / ` = { + render: () => , + play: async ({ canvasElement, step }) => { + const canvas = await openMenu(canvasElement); + await userEvent.click(canvas.getByRole('option', { name: /^chosen$/i })); + const input = await canvas.findByRole('combobox', { name: /search chosen values/i }); + + await step('both directions are listed at once, each under its heading', async () => { + const list = within(await canvas.findByRole('listbox', { name: /chosen values/i })); + await expect(list.getByRole('option', { name: /never chosen/i })).toBeInTheDocument(); + await expect(list.getByRole('option', { name: /10 times or more/i })).toBeInTheDocument(); + await expect(list.getByText('At most')).toBeInTheDocument(); + await expect(list.getByText('At least')).toBeInTheDocument(); + // The headings are not options — the option ids are positional. + await expect(list.getAllByRole('option')).toHaveLength(8); + await expect(canvas.queryByRole('radiogroup', { name: /direction/i })).not.toBeInTheDocument(); + }); + + await step('a bare number offers both directions', async () => { + await userEvent.type(input, '40'); + const options = within(await canvas.findByRole('listbox', { name: /chosen values/i })).getAllByRole('option'); + await expect(options.map((o) => o.textContent?.replace('custom', '').trim())).toEqual([ + '40 times or fewer', + '40 times or more', + ]); + await userEvent.clear(input); + }); + + await step('>N picks one side, and a strict comparison on a count reads as the next whole number', async () => { + await userEvent.type(input, '>40'); + const options = within(await canvas.findByRole('listbox', { name: /chosen values/i })).getAllByRole('option'); + await expect(options).toHaveLength(1); + await expect(options[0]).toHaveTextContent(/41 times or more/i); + await userEvent.keyboard('{Enter}'); + await waitFor(async () => { + await expect(within(canvasElement).getByLabelText('Chosen: 41 times or more')).toBeInTheDocument(); + }); + }); + + await step('the row reopens with the applied value selected, and the other side replaces it', async () => { + await userEvent.click(await canvas.findByRole('option', { name: /^chosen, 41 times or more$/i })); + const list = within(await canvas.findByRole('listbox', { name: /chosen values/i })); + await expect(list.getByRole('option', { name: /41 times or more/i })).toHaveAttribute('aria-selected', 'true'); + await userEvent.click(list.getByRole('option', { name: /never chosen/i })); + await waitFor(async () => { + await expect(within(canvasElement).getByLabelText('Chosen: Never chosen')).toBeInTheDocument(); + await expect(within(canvasElement).queryByLabelText('Chosen: 41 times or more')).not.toBeInTheDocument(); + }); + }); + }, +}; + +/** + * A row with no reverse reads one way: `Created` has the date-range picker for + * "created within", so `<30` explains where to go instead of offering a value. + */ +export const CreatedReadsOneWay: StoryObj = { + render: () => , + play: async ({ canvasElement }) => { + const canvas = await openMenu(canvasElement); + await userEvent.click(canvas.getByRole('option', { name: /^created$/i })); + await userEvent.type(await canvas.findByRole('combobox', { name: /search created values/i }), '<30'); + await expect(await canvas.findByText(/use the date range/i)).toBeInTheDocument(); + }, +}; + /** * A threshold pill behaves like a dimension pill: edit through the value * segment, clear through the ×. diff --git a/packages/web/src/components/lore/FilterMenu.tsx b/packages/web/src/components/lore/FilterMenu.tsx index 2c3a97bc1..f7e4a6df6 100644 --- a/packages/web/src/components/lore/FilterMenu.tsx +++ b/packages/web/src/components/lore/FilterMenu.tsx @@ -99,6 +99,18 @@ * Both props are optional together: a caller that passes neither gets the menu * without the section, exactly like `status`/`onStatusChange`. * + * ### Either direction, in the same list (migration 00113) + * Four of the five rows can be read both ways — `Chosen` as "5 times or fewer" + * (what a groomer asks) or "10 times or more" (what a reader looking for the + * lore that is actually used asks). Their value list holds BOTH directions as + * ordinary rows under two small headings (`At most` / `At least`, `More than` / + * `Within`), built by `retentionEntryRows`. It was briefly a toggle above the + * list, and that was a mode: half the values hid behind it, and it took a click + * and a Tab stop before the reader could even see them. As rows, every option is + * one arrow-key walk away. Typing works the same way — `40` offers both + * directions, `>40` / `<40` pick one. One row holds one direction at a time: + * picking a value clears the other (see `setRetentionCondition`). + * * ## Status lives here too, but it is NOT a dimension * A pinned "Status" row group sits above the search box at level 1, built * from `MEMORY_STATUSES`/`STATUS_LABELS`/`STATUS_ICONS` (`lib/status-filter.ts` @@ -141,6 +153,7 @@ import { } from 'lucide-react'; import { BottomSheet } from '@/components/ui/BottomSheet'; import { + CHROME_HEIGHT, anchoredPosition, type AnchoredPosition, } from '@/lib/anchored-position'; @@ -161,9 +174,11 @@ import { RETENTION_CONDITION_BOUNDS, requireRetentionField, retentionConditionsCount, + retentionEntryRows, + retentionEntryValue, retentionFieldMatches, - retentionValueRows, setRetentionCondition, + type PolicyRetentionField, type RetentionConditions, type RetentionField, } from '@/lib/retention-filter'; @@ -201,14 +216,19 @@ export const FIELD_ICONS: Record = { * Each names its DATA rather than its condition: a calendar-plus for when a * lesson was written, a clock-history for when one was last opened, a repeat * for how often it recurred, a download for how often it was delivered, a - * pointer for how often something chose it. + * pointer for how often something chose it. A reverse direction (00113) reads + * the same data, so it wears the same icon as its row. */ export const RETENTION_FIELD_ICONS: Record = { minAgeDays: CalendarPlus, unseenDays: History, + openedWithinDays: History, maxSeenCount: Repeat, + minSeenCount: Repeat, maxReadCount: Download, + minReadCount: Download, maxOpenedCount: MousePointerClick, + minOpenedCount: MousePointerClick, }; /** @@ -217,7 +237,7 @@ export const RETENTION_FIELD_ICONS: Record = { * addresses ONE array — splitting them would mean two index spaces and an * arrow key that has to know which half it is in. */ -type MenuRootRow = RootSuggestion | { kind: 'retention'; field: RetentionField }; +type MenuRootRow = RootSuggestion | { kind: 'retention'; field: PolicyRetentionField }; /** * Which value list is showing, or `null` for the level-1 row list. A @@ -226,7 +246,9 @@ type MenuRootRow = RootSuggestion | { kind: 'retention'; field: RetentionField } */ type MenuLevel = | { kind: 'filter'; field: FilterField } - | { kind: 'retention'; field: RetentionField }; + // A threshold's level is its MENU ROW; the rows inside carry their own + // direction (see `retentionEntryRows`). + | { kind: 'retention'; field: PolicyRetentionField }; /** * This menu's own list height, overriding the shared default the way @@ -239,7 +261,14 @@ type MenuLevel = * takes the smaller of this and the space actually available, so a short viewport * shrinks the list rather than pushing it off-screen. */ -const FILTER_MENU_SIZE = { maxListHeight: 400 } as const; +const FILTER_MENU_SIZE = { + maxListHeight: 400, + // The shared chrome figure covers the search box, the footer and a level-two + // breadcrumb. Level one also carries the Status row (~40px the default did + // not count), which on a short viewport let the menu run past the bottom edge. + // Measured once on open rather than per level, so this budgets for it. + chromeHeight: CHROME_HEIGHT + 40, +} as const; const LISTBOX_ID = 'filter-menu-listbox'; const OPTION_ID_PREFIX = 'filter-menu-option-'; @@ -409,7 +438,9 @@ export function FilterMenu({ // responds to is worse than no menu. useEffect(() => { if (!openAtRetentionField || !retentionEnabled) return; - openAtLevel({ kind: 'retention', field: openAtRetentionField }); + // A pill names a DIRECTION (`minOpenedCount`); the menu opens on its row, + // where the applied value is selected inside the right group. + openAtLevel({ kind: 'retention', field: requireRetentionField(openAtRetentionField).entry }); onOpenAtRetentionFieldHandled?.(); }, [openAtRetentionField, retentionEnabled, onOpenAtRetentionFieldHandled, openAtLevel]); @@ -436,10 +467,7 @@ export function FilterMenu({ }, [facets, field, filters, query]); const retentionRows = useMemo( - () => - retentionField - ? retentionValueRows(retentionField, query, retention?.[retentionField]) - : [], + () => (retentionField ? retentionEntryRows(retentionField, query, retention ?? {}) : []), [retentionField, query, retention], ); @@ -578,10 +606,9 @@ export function FilterMenu({ function commitRetentionRow(index: number) { const row = retentionRows[index]; if (!row || !retentionField || !retention || !onRetentionChange) return; - const cleared = retention[retentionField] === row.value; - onRetentionChange( - setRetentionCondition(retention, retentionField, cleared ? undefined : row.value), - ); + // The ROW's field, not the level's: the list holds both directions. + const cleared = retention[row.field] === row.value; + onRetentionChange(setRetentionCondition(retention, row.field, cleared ? undefined : row.value)); if (entryDepth === 1) { closeMenu(); return; @@ -745,11 +772,16 @@ export function FilterMenu({ // so the only way to see nothing here is to type something that is not a legal // value. Saying so, with the range, is the difference between "this filter is // broken" and "3651 is too many days". - const retentionEmptyCopy = retentionField - ? `“${query.trim()}” is not a value here — type a whole number from ${ - RETENTION_CONDITION_BOUNDS[retentionField].min - } to ${RETENTION_CONDITION_BOUNDS[retentionField].max}.` - : ''; + const retentionEmptyCopy = !retentionField + ? '' + : retentionDescriptor?.opposite !== undefined + ? `“${query.trim()}” is not a value here — type a whole number, or >N / {retentionRows.map((row, i) => { - const isSelected = retention?.[retentionField] === row.value; + const isSelected = retention?.[row.field] === row.value; + const rowDescriptor = requireRetentionField(row.field); + // A heading opens each direction's group, between rows rather than + // as an entry in the array — the option ids are positional (see the + // `Age & activity` heading below for the same reasoning). + const heading = + retentionDescriptor.opposite !== undefined && retentionRows[i - 1]?.field !== row.field + ? rowDescriptor.directionLabel + : undefined; return ( - + {rowDescriptor.formatValue(row.value)} + {row.custom && ( + // Labelled so a typed threshold is visibly a typed one and not + // a preset the reader misremembers next time. + + custom + + )} + + ); })} {/* What the threshold actually tests, once under the list rather than once per row: the rows are numbers, and a number without its rule is the thing readers got wrong about `Delivered` vs `Chosen`. */}

- {retentionDescriptor.hint} + {retentionDescriptor.entryHint ?? retentionDescriptor.hint}

) : field ? ( @@ -875,7 +924,13 @@ export function FilterMenu({ if (row.kind === 'retention') { const entry = requireRetentionField(row.field); const Icon = RETENTION_FIELD_ICONS[row.field]; - const value = retention?.[row.field]; + // Either direction — the badge has to name what is applied, and + // `Chosen: 10 times or more` is just as applied as `Never chosen`. + const applied = retention ? retentionEntryValue(retention, row.field) : null; + const value = applied?.value; + const appliedLabel = applied + ? requireRetentionField(applied.field).formatValue(applied.value) + : undefined; // The heading is rendered BETWEEN rows rather than as an entry in // `rootRows`, because `activeIndex` and the `role="option"` ids are // positional — a non-selectable element in the array would shift @@ -903,11 +958,7 @@ export function FilterMenu({ // The chosen value is a badge, and a badge is aria-hidden // decoration — fold it into the name so a screen reader hears // "Chosen, never chosen" rather than just "Chosen". - aria-label={ - value !== undefined - ? `${entry.label}, ${entry.formatValue(value)}` - : undefined - } + aria-label={appliedLabel !== undefined ? `${entry.label}, ${appliedLabel}` : undefined} onClick={() => commitRootRow(i)} onMouseEnter={() => setActiveIndex(i)} className={[ @@ -936,7 +987,7 @@ export function FilterMenu({ // the phrase that carries no information. className="max-w-36 truncate rounded-full bg-[var(--color-accent-subtle)] px-1.5 text-[10px] font-semibold text-[var(--color-accent)]" > - {entry.formatValue(value)} + {appliedLabel} )} ${retentionDescriptor.typeExample} or <${retentionDescriptor.typeExample}…` + : retentionDescriptor ? 'Pick one, or type a number…' : field ? descriptor?.searchPlaceholder diff --git a/packages/web/src/components/lore/LoreExplorer.tsx b/packages/web/src/components/lore/LoreExplorer.tsx index 32778826a..2bdb24dad 100644 --- a/packages/web/src/components/lore/LoreExplorer.tsx +++ b/packages/web/src/components/lore/LoreExplorer.tsx @@ -50,6 +50,12 @@ * narrowing. Resolves to no conditions while the `retention-policies` flag is * off — the whole feature is gated together with its Settings → Retention * Policies destination, which 404s while the flag is off. + * Four of the five also read the other way (migration 00113 — "chosen at + * least N times", "opened within N days"); those reverse directions are + * Explorer-only, so "Create retention policy" is hidden while one is set. + * - `sort` param: the list order (`lib/explorer-sort.ts`) — Newest (absent, + * the default) or Most chosen (`opened_count`, migration 00113). Shareable; + * an unknown value falls back to Newest. * - `owner` param: the superseded ownership shorthand from the old * client-side owner bar. Still READ so old links (and pre-change accept-invite * deep links) land; never written. `resolveFilters` folds a `'personal'` value @@ -139,7 +145,14 @@ import { type FilterOperator, } from '@/lib/filters'; import { FilterMenuTrigger, FilterPillRow } from './FilterBar'; +import { SortSelect } from './SortSelect'; import { + explorerSortParamValue, + resolveExplorerSort, + type ExplorerSort, +} from '@/lib/explorer-sort'; +import { + hasExplorerOnlyConditions, hasRetentionConditions, normalizeRetentionConditions, retentionConditionsParamValue, @@ -238,6 +251,8 @@ function ControlRow({ onRetentionChange, editingRetentionField, onEditRetentionField, + sort, + onSortChange, }: { variant: 'desktop' | 'mobile'; search: string; @@ -264,6 +279,9 @@ function ControlRow({ onRetentionChange: (next: RetentionConditions) => void; editingRetentionField: RetentionField | null; onEditRetentionField: (field: RetentionField | null) => void; + /** The list order — see `SortSelect` for why it is not inside the Filter menu. */ + sort: ExplorerSort; + onSortChange: (sort: ExplorerSort) => void; }) { const desktop = variant === 'desktop'; @@ -307,6 +325,7 @@ function ControlRow({ active={dateActive} className="shrink-0" /> + ); } @@ -542,6 +561,11 @@ export function LoreExplorer({ scopes, heatmapData }: LoreExplorerProps) { }, [setRawRetention, retentionConditions, filters.length], ); + // "Create retention policy" hands this view to a policy, so it is offered only + // when a policy could reproduce it: the flag is on (the destination 404s + // otherwise) and no Explorer-only reverse direction is set. + const canCreatePolicy = + retentionPoliciesEnabled && !hasExplorerOnlyConditions(retentionConditions); // Which threshold the menu should open at, set by a retention pill's value // segment — the `editingField` above, for the other half of the bar. const [editingRetentionField, setEditingRetentionField] = useState(null); @@ -611,6 +635,23 @@ export function LoreExplorer({ scopes, heatmapData }: LoreExplorerProps) { const status = resolveStatus(rawStatus, legacyArchived); const showArchived = isArchivedView(status); + // URL-backed list order — scoped to /lore, absent for the default (Newest) so + // an unsorted link is byte-identical to one minted before sorting existed. + // `resolveExplorerSort` turns a stale or hand-edited value into the default + // rather than a 400 from the list route. + const [rawSort, setRawSort] = useUrlState('sort', null, { + cleanOnPathname: '/lore', + }); + const sort = resolveExplorerSort(rawSort); + const handleSortChange = useCallback( + (next: ExplorerSort) => { + if (next === sort) return; + track({ name: 'lore.sort_changed', sort: next }); + setRawSort(explorerSortParamValue(next)); + }, + [sort, setRawSort], + ); + // Paginated lesson list — server-side filtered by scope / search / range / status. const { data, @@ -625,6 +666,7 @@ export function LoreExplorer({ scopes, heatmapData }: LoreExplorerProps) { range: resolvedRange, filters, retentionConditions, + sort, showArchived, expiringWithinDays: expiringWithinDays(status), }); @@ -1488,6 +1530,8 @@ export function LoreExplorer({ scopes, heatmapData }: LoreExplorerProps) { retentionEnabled={retentionPoliciesEnabled} retentionConditions={retentionConditions} onRetentionChange={setRetentionConditions} + sort={sort} + onSortChange={handleSortChange} />
{renderResults()}
@@ -1530,6 +1576,8 @@ export function LoreExplorer({ scopes, heatmapData }: LoreExplorerProps) { retentionEnabled={retentionPoliciesEnabled} retentionConditions={retentionConditions} onRetentionChange={setRetentionConditions} + sort={sort} + onSortChange={handleSortChange} />
@@ -1542,7 +1590,7 @@ export function LoreExplorer({ scopes, heatmapData }: LoreExplorerProps) { retention={retentionConditions} onRetentionChange={setRetentionConditions} onEditRetentionField={setEditingRetentionField} - onCreatePolicy={retentionPoliciesEnabled ? handleCreatePolicy : undefined} + onCreatePolicy={canCreatePolicy ? handleCreatePolicy : undefined} />
diff --git a/packages/web/src/components/lore/RetentionPill.tsx b/packages/web/src/components/lore/RetentionPill.tsx index 4517ac1ea..d2475d2cd 100644 --- a/packages/web/src/components/lore/RetentionPill.tsx +++ b/packages/web/src/components/lore/RetentionPill.tsx @@ -16,9 +16,12 @@ * The **operator segment is absent, not empty**. A dimension can include or * exclude, so its middle segment is a real choice; a threshold's comparison is * baked into the condition itself (`min_age_days` is always "at least", - * `max_opened_count` always "at most"), and the value's own wording carries it — - * "More than 30 days ago", "Never chosen". Rendering an inert `is` segment would - * put a third of the pill's width into a word that answers nothing. + * `max_opened_count` always "at most", `min_opened_count` always "at least"), + * and the value's own wording carries it — "More than 30 days ago", "Never + * chosen", "10 times or more". Rendering an inert `is` segment would put a + * third of the pill's width into a word that answers nothing. Flipping a + * direction happens in the menu's value list, which the value segment opens on + * the applied direction. * * The type segment stays inert for the same reason it is in `FilterPill`: * changing a threshold's field would invalidate the value it holds, so the diff --git a/packages/web/src/components/lore/SortSelect.tsx b/packages/web/src/components/lore/SortSelect.tsx new file mode 100644 index 000000000..f0b0b164b --- /dev/null +++ b/packages/web/src/components/lore/SortSelect.tsx @@ -0,0 +1,106 @@ +'use client'; + +/** + * SortSelect — the Lore Explorer's list order (`?sort=`, `lib/explorer-sort.ts`). + * + * A native ` onChange(e.target.value as ExplorerSort)} + aria-label={`Sort memories: ${EXPLORER_SORT_LABELS[value]}`} + className="absolute inset-0 cursor-pointer appearance-none opacity-0" + > + {options} + + + ); + } + + return ( +
+ + + +
+ ); +} diff --git a/packages/web/src/content/docs/grooming.mdx b/packages/web/src/content/docs/grooming.mdx index 3dff0011b..76b344428 100644 --- a/packages/web/src/content/docs/grooming.mdx +++ b/packages/web/src/content/docs/grooming.mdx @@ -148,6 +148,41 @@ conditions AND filter bar to Settings → Retention Policies, which opens the Ne policy dialog pre-filled with all of it, using the identical filter control to edit it further. +### Reading the other way: the lore that IS used + +Four of the entries also answer the opposite question. Their value list shows +**both directions at once**, as two groups of presets — **At most / At least** +for the three counters, **More than / Within** for Last agent open. Typing works +the same way: a bare number such as `40` offers both `40 times or fewer` and +`40 times or more`, while `>40` or `<40` picks one side (`>=` and `<=` work +too). On a count a strict comparison means the next whole number, so `>40` +reads — and applies — as `41 times or more`; on days it does not, so `>30` under +Last agent open is `More than 30 days ago`. + +| Menu entry | Reverse condition | What it tests | +|---|---|---| +| Last agent open → Within | `opened_within_days` | Last deliberately fetched within N days. A lesson never opened does **not** match, however new | +| Recurrence → At least | `min_seen_count` | Written N times or more | +| Delivered → At least | `min_read_count` | Handed to an agent N times or more | +| Chosen → At least | `min_opened_count` | Deliberately fetched N times or more — the lore agents actually reach for | + +A row holds one direction at a time: picking `Chosen → 10 times or more` replaces +`Chosen: 5 times or fewer` rather than combining into an empty range. The pill +carries the direction in its wording (`Chosen: 10 times or more`), and the +counts, stat cards and matrix narrow exactly as they do for the retention side. + +These reverse directions are for **exploring**, not for policies — a retention +policy cannot carry them, so **Create retention policy** is hidden while one is +set rather than saving a policy that would archive a different set from the one +on screen. + +To see the most-used lore first without picking a threshold at all, set the +**Sort** control next to the date picker to **Most chosen**: the list orders by +deliberate fetches, highest first. The order is kept in the page URL like the +filters, so a sorted view is shareable; **Newest** is the default. Over the +REST API the same order is `sort=opened_count` on `GET /memories` and +`POST /memories/list`. + diff --git a/packages/web/src/lib/analytics/lore-events.spec.ts b/packages/web/src/lib/analytics/lore-events.spec.ts index 7dc13dfd9..d265632a8 100644 --- a/packages/web/src/lib/analytics/lore-events.spec.ts +++ b/packages/web/src/lib/analytics/lore-events.spec.ts @@ -134,6 +134,25 @@ describe('retentionFilterChange', () => { }); }); + // 00113: setting one direction clears its opposite in the same write. That is + // one change to one row, so it must still name the field the reader picked. + it('names the field a direction switch set, though its opposite moved too', () => { + expect(retentionFilterChange({ maxOpenedCount: 5 }, { minOpenedCount: 10 })).toEqual({ + action: 'retention-set', + field: 'minOpenedCount', + }); + expect(retentionFilterChange({ openedWithinDays: 7 }, { unseenDays: 90 })).toEqual({ + action: 'retention-set', + field: 'unseenDays', + }); + }); + + it('still reports clearing BOTH directions of a row as a multi-field remove', () => { + expect(retentionFilterChange({ maxSeenCount: 5, minSeenCount: 2 }, {})).toEqual({ + action: 'retention-remove', + }); + }); + // The whole reason this returns null: "Clear all" calls the retention setter // with `{}` even on a bar that never had a threshold, and emitting there // would double-count every filter clear. diff --git a/packages/web/src/lib/analytics/lore-events.ts b/packages/web/src/lib/analytics/lore-events.ts index 5ab8d8f88..c758e5f7f 100644 --- a/packages/web/src/lib/analytics/lore-events.ts +++ b/packages/web/src/lib/analytics/lore-events.ts @@ -35,6 +35,7 @@ import { RETENTION_FIELDS, + requireRetentionField, type RetentionConditions, type RetentionField, } from '@/lib/retention-filter'; @@ -232,6 +233,8 @@ export function rangeTelemetry(range: TimeRange, nowIso: string): RangeTelemetry * * A multi-field change reports no `field`: naming one of several would be a * guess, and an omitted attribute is the honest encoding of "more than one". + * The exception is one row's direction switch (00113), which moves a field and + * its opposite together and is reported as setting the one now held. */ export function retentionFilterChange( prev: RetentionConditions, @@ -249,6 +252,14 @@ export function retentionFilterChange( field: first, }; } + // One row's direction switch is ONE change: setting `minOpenedCount` clears + // `maxOpenedCount` in the same write (setRetentionCondition), so two fields + // move. Name the one now set. Both cleared stays a plain multi-field remove. + const [a, b] = changed; + if (changed.length === 2 && a !== undefined && b !== undefined && requireRetentionField(a).opposite === b) { + const set = [a, b].filter((field) => next[field] !== undefined); + if (set.length === 1 && set[0] !== undefined) return { action: 'retention-set', field: set[0] }; + } const anySet = changed.some((field) => next[field] !== undefined); return { action: anySet ? 'retention-set' : 'retention-remove' }; } diff --git a/packages/web/src/lib/analytics/track.ts b/packages/web/src/lib/analytics/track.ts index 1bfd4221c..59d801f39 100644 --- a/packages/web/src/lib/analytics/track.ts +++ b/packages/web/src/lib/analytics/track.ts @@ -46,6 +46,7 @@ * - `lore.memory_archive_toggled` — a memory was archived or restored. * - `lore.filter_changed` — any filter-bar mutation, thresholds included. * - `lore.status_changed` — Active / Archived / Expiring. + * - `lore.sort_changed` — the list order (Newest / Most chosen). * - `lore.search_committed` — the debounced search settled. * - `lore.scope_selected` — a scope chip (or Browse-all row) was picked. * - `lore.range_changed` — any of the FIVE controls that write `?range=`. @@ -86,6 +87,7 @@ import type { ButtonSize, ButtonVariant } from '@/lib/button-styles'; import type { FilterField, FilterOperator } from '@/lib/filters'; import type { LessonUtility } from '@/lib/lesson-utility'; import type { MemoryStatus } from '@/lib/status-filter'; +import type { ExplorerSort } from '@/lib/explorer-sort'; import type { RetentionField } from '@/lib/retention-filter'; import { dwellBucket, type ContentType, type ScrollMilestone } from './reading'; @@ -299,6 +301,8 @@ export type AnalyticsEvent = retentionCount: number; } | { name: 'lore.status_changed'; status: MemoryStatus } + /** The list order picked — a closed set (`lib/explorer-sort.ts`), never free text. */ + | { name: 'lore.sort_changed'; sort: ExplorerSort } | { name: 'lore.search_committed'; /** @@ -512,6 +516,8 @@ function toAttributes(event: AnalyticsEvent): Record { } case 'lore.status_changed': return { 'lorekit.lore.status': event.status }; + case 'lore.sort_changed': + return { 'lorekit.lore.sort': event.sort }; case 'lore.search_committed': // A measure, and unconditional: a 0 is the reader CLEARING the box, which // is as much a fact as typing into it. diff --git a/packages/web/src/lib/explorer-sort.spec.ts b/packages/web/src/lib/explorer-sort.spec.ts new file mode 100644 index 000000000..6e5c8f0d9 --- /dev/null +++ b/packages/web/src/lib/explorer-sort.spec.ts @@ -0,0 +1,43 @@ +import { describe, it, expect } from 'vitest'; +import { MemorySortSchema } from '@lorekit/schemas/memory'; +import { + DEFAULT_EXPLORER_SORT, + EXPLORER_SORTS, + EXPLORER_SORT_LABELS, + explorerSortParamValue, + resolveExplorerSort, +} from './explorer-sort'; + +describe('resolveExplorerSort', () => { + it('keeps a known order', () => { + expect(resolveExplorerSort('opened_count')).toBe('opened_count'); + expect(resolveExplorerSort('created_at')).toBe('created_at'); + }); + + it('falls back to Newest for an absent, unknown or non-string value', () => { + for (const raw of [null, undefined, '', 'read_count', 'updated_at', 42, {}]) { + expect(resolveExplorerSort(raw)).toBe(DEFAULT_EXPLORER_SORT); + } + }); +}); + +describe('explorerSortParamValue', () => { + it('drops the param for the default and keeps it otherwise', () => { + expect(explorerSortParamValue('created_at')).toBeNull(); + expect(explorerSortParamValue('opened_count')).toBe('opened_count'); + }); +}); + +describe('EXPLORER_SORTS', () => { + it('offers only orders the API accepts', () => { + // An option the list route rejected would be a 400 rendered as "Failed to + // load memories" the moment a reader picked it. + for (const sort of EXPLORER_SORTS) { + expect(MemorySortSchema.safeParse(sort).success, sort).toBe(true); + } + }); + + it('labels every option', () => { + for (const sort of EXPLORER_SORTS) expect(EXPLORER_SORT_LABELS[sort].length).toBeGreaterThan(0); + }); +}); diff --git a/packages/web/src/lib/explorer-sort.ts b/packages/web/src/lib/explorer-sort.ts new file mode 100644 index 000000000..628839589 --- /dev/null +++ b/packages/web/src/lib/explorer-sort.ts @@ -0,0 +1,48 @@ +/** + * The Lore Explorer's list order — the `?sort=` URL param. + * + * Two orders, both descending: + * + * - `created_at` — **Newest**, the default and the order the list has always + * had (`listMemories` used to hard-code it so a backdated migration lands at + * its original position). + * - `opened_count` — **Most chosen**, migration 00113: the lessons agents + * deliberately fetched the most, first. It is the direct answer to "which + * lore is actually used?", where a `Chosen: N times or more` threshold makes + * the reader guess an N first. + * + * `updated_at` is a legal API sort but is not offered: a re-write bumps it, so + * as a list order it mostly re-states recurrence, which `Recurrence` already + * filters on. + * + * Pure, so the URL handling is unit-tested without a browser. + */ + +export const EXPLORER_SORTS = ['created_at', 'opened_count'] as const; +export type ExplorerSort = (typeof EXPLORER_SORTS)[number]; + +/** The order an absent `?sort=` means. */ +export const DEFAULT_EXPLORER_SORT: ExplorerSort = 'created_at'; + +/** The visible option text. Short: the control shares a row with search, Filter and the date picker. */ +export const EXPLORER_SORT_LABELS: Record = { + created_at: 'Newest', + opened_count: 'Most chosen', +}; + +/** + * Read a `?sort=` value. Anything that is not one of {@link EXPLORER_SORTS} — + * a hand-edited link, a value from a later version — falls back to the default + * rather than reaching the API, where it would be a 400 and an empty list. + */ +export function resolveExplorerSort(raw: unknown): ExplorerSort { + return EXPLORER_SORTS.includes(raw as ExplorerSort) ? (raw as ExplorerSort) : DEFAULT_EXPLORER_SORT; +} + +/** + * What to write to `?sort=` — `null` drops the param for the default, so an + * unsorted Explorer link stays exactly as short as it was before sorting existed. + */ +export function explorerSortParamValue(sort: ExplorerSort): ExplorerSort | null { + return sort === DEFAULT_EXPLORER_SORT ? null : sort; +} diff --git a/packages/web/src/lib/lore.ts b/packages/web/src/lib/lore.ts index bac3ecdb8..421628c94 100644 --- a/packages/web/src/lib/lore.ts +++ b/packages/web/src/lib/lore.ts @@ -25,6 +25,7 @@ import { dateRangeBounds, type DateRangeInput } from '@/lib/pagination/filters'; import type { LessonEntry } from '@/components/lore/LessonCard'; import { normalizeTags } from '@/lib/tag-filter'; import { filtersToBody, normalizeFilters, type Filter } from '@/lib/filters'; +import { DEFAULT_EXPLORER_SORT, type ExplorerSort } from '@/lib/explorer-sort'; import { retentionConditionsToListBody, type RetentionConditions, @@ -193,6 +194,13 @@ export interface MemoryFilters { * what that policy would catch. Absent/empty means no narrowing. */ retentionConditions?: RetentionConditions; + /** + * The list order (`lib/explorer-sort.ts`) — `created_at` (Newest, the + * default) or `opened_count` (Most chosen, migration 00113). A cursor is only + * valid for the order that minted it; the route restarts at page one if the + * two disagree, and the Explorer's query key changes with the sort anyway. + */ + sort?: ExplorerSort; /** Page size, default 50, hard max 100. */ pageSize?: number; /** Opaque keyset cursor from a previous page's `nextCursor`. */ @@ -231,10 +239,10 @@ const EMPTY_PAGE: MemoryPage = { rows: [], nextCursor: null, hasMore: false }; * List a keyset page of the memories the caller can see, newest first, with * optional combinable filters (scope / substring / date interval / labels). * - * Ordering is `created_at desc` — `sort: 'created_at'` — not the route's - * `updated_at` default: a memory migrated with a backdated `created_at` belongs - * at its original position in the Explorer, which is the order the list has - * always been in. + * Ordering defaults to `created_at desc` — `sort: 'created_at'` — not the + * route's `updated_at` default: a memory migrated with a backdated `created_at` + * belongs at its original position in the Explorer, which is the order the list + * has always been in. `filters.sort` swaps in `opened_count` ("Most chosen"). * * Sent over `POST /memories/list` rather than `GET /memories`: the filter bar's * dimensions are unbounded, and the query transport caps each one at 2048 @@ -265,7 +273,7 @@ export async function listMemories(filters: MemoryFilters = {}): Promise { const args: MemoryFilters = { @@ -633,6 +640,7 @@ export function useMemories(filters: UseMemoriesFilters) { range: filters.range, filters: bar, retentionConditions: filters.retentionConditions, + sort: filters.sort, cursor: pageParam as string | null, showArchived: filters.showArchived, expiringWithinDays: filters.expiringWithinDays, diff --git a/packages/web/src/lib/retention-filter.spec.ts b/packages/web/src/lib/retention-filter.spec.ts index a567fdb82..b0b382afa 100644 --- a/packages/web/src/lib/retention-filter.spec.ts +++ b/packages/web/src/lib/retention-filter.spec.ts @@ -11,7 +11,13 @@ import { NO_RETENTION_CONDITIONS, RETENTION_CONDITION_BOUNDS, RETENTION_FIELDS, + RETENTION_MENU_FIELDS, filtersToGroomDimensionFilters, + hasExplorerOnlyConditions, + parseRetentionQuery, + policyRetentionConditions, + retentionEntryRows, + retentionEntryValue, groomConditionsToFilters, hasRetentionConditions, normalizeRetentionConditions, @@ -69,6 +75,16 @@ describe('normalizeRetentionConditions', () => { it('drops an unrecognised key', () => { expect(normalizeRetentionConditions({ minAgeDays: 90, bogus: 1 })).toEqual({ minAgeDays: 90 }); }); + + it('keeps the reverse directions (00113), bounded from 1', () => { + // Additive keys, so every pre-00113 `?retention=` link still resolves to + // exactly what it meant before. + expect( + normalizeRetentionConditions({ openedWithinDays: 7, minSeenCount: 2, minReadCount: 100, minOpenedCount: 10 }), + ).toEqual({ openedWithinDays: 7, minSeenCount: 2, minReadCount: 100, minOpenedCount: 10 }); + // A floor of 0 narrows nothing, so it is dropped like any out-of-range value. + expect(normalizeRetentionConditions({ minOpenedCount: 0, openedWithinDays: 0 })).toEqual({}); + }); }); describe('hasRetentionConditions / retentionConditionsCount', () => { @@ -97,6 +113,26 @@ describe('hasRetentionConditions / retentionConditionsCount', () => { // falsy-check regression here would silently disable the condition. expect(hasRetentionConditions({ maxOpenedCount: 0 })).toBe(true); }); + + it('counts a reverse direction like any other condition', () => { + expect(hasRetentionConditions({ minOpenedCount: 10 })).toBe(true); + expect(retentionConditionsCount({ maxSeenCount: 1, minOpenedCount: 10, openedWithinDays: 7 })).toBe(3); + }); +}); + +describe('hasExplorerOnlyConditions / policyRetentionConditions', () => { + it('is true only for a condition a retention policy cannot carry', () => { + expect(hasExplorerOnlyConditions({ minAgeDays: 90, maxOpenedCount: 0 })).toBe(false); + for (const field of ['openedWithinDays', 'minSeenCount', 'minReadCount', 'minOpenedCount'] as const) { + expect(hasExplorerOnlyConditions({ [field]: 1 }), field).toBe(true); + } + }); + + it('keeps just the five policy conditions', () => { + expect( + policyRetentionConditions({ minAgeDays: 90, maxOpenedCount: 0, minReadCount: 100, openedWithinDays: 7 }), + ).toEqual({ minAgeDays: 90, maxOpenedCount: 0 }); + }); }); describe('retentionConditionsParamValue', () => { @@ -152,6 +188,27 @@ describe('retentionConditionsToListBody / retentionConditionsToGroomConditions', expect(GroomConditionsSchema.safeParse(groom).success).toBe(true); expect(groom).toEqual({ min_age_days: 90, unseen_days: 30, max_seen_count: 1 }); }); + + it('maps the reverse directions to their own wire fields on the LIST body', () => { + const body = retentionConditionsToListBody({ + openedWithinDays: 7, + minSeenCount: 2, + minReadCount: 100, + minOpenedCount: 10, + }); + expect(body).toEqual({ opened_within_days: 7, min_seen_count: 2, min_read_count: 100, min_opened_count: 10 }); + // Read back off the parsed output: the schema is not `.strict()`, so a + // success-only check would pass a route that silently dropped them. + const parsed = ListMemoriesBodySchema.parse(body); + expect([parsed.opened_within_days, parsed.min_seen_count, parsed.min_read_count, parsed.min_opened_count]).toEqual([ + 7, 2, 100, 10, + ]); + }); + + it('never hands a reverse direction to a POLICY — GroomConditions has no field for it', () => { + const groom = retentionConditionsToGroomConditions({ maxOpenedCount: 0, minOpenedCount: 10, openedWithinDays: 7 }); + expect(groom).toEqual({ max_opened_count: 0 }); + }); }); describe('filtersToGroomDimensionFilters', () => { @@ -320,6 +377,18 @@ describe('retentionConditionsToAggregateBody', () => { }); }); + it('carries the reverse directions to all three aggregates, so the counts describe the list (00113)', () => { + const body = retentionConditionsToAggregateBody({ openedWithinDays: 7, minOpenedCount: 10 }); + expect(body).toEqual({ opened_within_days: 7, min_opened_count: 10 }); + for (const parsed of [ + ListFacetsBodySchema.parse(body), + ActivityBodySchema.parse(body), + PivotBodySchema.parse({ row: 'host', col: 'kind', ...body }), + ]) { + expect([parsed.opened_within_days, parsed.min_opened_count]).toEqual([7, 10]); + } + }); + it('emits fields ALL THREE aggregate schemas actually CARRY', () => { const body = retentionConditionsToAggregateBody({ minAgeDays: 7, @@ -384,11 +453,28 @@ describe('retentionConditionsToAggregateBody', () => { describe('RETENTION_FIELDS', () => { it('covers every condition exactly once, and in a stable order', () => { - // The menu's row order, the pill order and `retentionConditionsPhrase`'s - // fragment order are all this one array, so a field added to - // `RetentionConditions` without an entry here is a condition the URL can - // carry and the UI can never show or clear. + // The pill order and `retentionConditionsPhrase`'s fragment order are this + // one array, so a field added to `RetentionConditions` without an entry + // here is a condition the URL can carry and the UI can never show or clear. + // Each reverse direction sits right after the row it reverses. expect(RETENTION_FIELDS.map((d) => d.field)).toEqual([ + 'minAgeDays', + 'unseenDays', + 'openedWithinDays', + 'maxSeenCount', + 'minSeenCount', + 'maxReadCount', + 'minReadCount', + 'maxOpenedCount', + 'minOpenedCount', + ]); + expect(RETENTION_FIELDS.map((d) => d.field).sort()).toEqual( + Object.keys(RETENTION_CONDITION_BOUNDS).sort(), + ); + }); + + it('lists five menu rows — one per metadata row, never one per direction', () => { + expect(RETENTION_MENU_FIELDS).toEqual([ 'minAgeDays', 'unseenDays', 'maxSeenCount', @@ -397,6 +483,37 @@ describe('RETENTION_FIELDS', () => { ]); }); + it('pairs each reverse direction with its row symmetrically, under the same label', () => { + for (const d of RETENTION_FIELDS) { + if (d.opposite === undefined) continue; + const other = requireRetentionField(d.opposite); + expect(other.opposite, d.field).toBe(d.field); + expect(other.entry, d.field).toBe(d.entry); + expect(other.label, d.field).toBe(d.label); + // Two group headings reading the same would be unreadable. + expect(other.directionLabel, d.field).not.toBe(d.directionLabel); + // Opposites keep opposite sides of N — that is what `<` / `>` route on. + expect(other.bound, d.field).not.toBe(d.bound); + expect(other.discrete, d.field).toBe(d.discrete); + } + // A row whose list holds both directions needs a hint that covers both. + for (const field of RETENTION_MENU_FIELDS) { + const d = requireRetentionField(field); + if (d.opposite !== undefined) expect(d.entryHint, field).toBeTruthy(); + } + // Created has no reverse: the date range picker already answers "created + // within", and a second spelling of it would be two controls for one fact. + expect(requireRetentionField('minAgeDays').opposite).toBeUndefined(); + }); + + it('words the reverse directions as "or more" / "within", so a pill carries its direction', () => { + expect(requireRetentionField('minOpenedCount').formatValue(10)).toBe('10 times or more'); + expect(requireRetentionField('minOpenedCount').formatValue(1)).toBe('At least once'); + expect(requireRetentionField('minReadCount').formatValue(500)).toBe('500 times or more'); + expect(requireRetentionField('openedWithinDays').formatValue(7)).toBe('Within 7 days'); + expect(requireRetentionField('openedWithinDays').formatValue(1)).toBe('Within 1 day'); + }); + it('offers only in-bounds presets', () => { // A preset that `normalizeRetentionConditions` would drop is a row that // applies a filter and then silently reverts on the next read of the URL. @@ -431,9 +548,16 @@ describe('requireRetentionField', () => { }); describe('retentionFieldMatches', () => { - it('offers every entry, in order, with no query', () => { - expect(retentionFieldMatches('')).toEqual(RETENTION_FIELDS.map((d) => d.field)); - expect(retentionFieldMatches(' ')).toEqual(RETENTION_FIELDS.map((d) => d.field)); + it('offers every menu row, in order, with no query', () => { + expect(retentionFieldMatches('')).toEqual(RETENTION_MENU_FIELDS); + expect(retentionFieldMatches(' ')).toEqual(RETENTION_MENU_FIELDS); + }); + + it('finds a row by the reverse question too', () => { + // "most used" is the question 00113 exists for; it must land on the row + // that answers it, never on a second row per direction. + expect(retentionFieldMatches('most')).toEqual(['maxOpenedCount']); + expect(retentionFieldMatches('recent')).toEqual(['unseenDays']); }); it('ranks label hits above keyword hits', () => { @@ -529,7 +653,141 @@ describe('retentionValueRows', () => { }); }); +describe('retentionEntryValue', () => { + it('reports whichever direction a row holds', () => { + expect(retentionEntryValue({ maxOpenedCount: 0 }, 'maxOpenedCount')).toEqual({ field: 'maxOpenedCount', value: 0 }); + expect(retentionEntryValue({ minOpenedCount: 10 }, 'maxOpenedCount')).toEqual({ field: 'minOpenedCount', value: 10 }); + expect(retentionEntryValue({ openedWithinDays: 7 }, 'unseenDays')).toEqual({ field: 'openedWithinDays', value: 7 }); + expect(retentionEntryValue({}, 'maxOpenedCount')).toBeNull(); + expect(retentionEntryValue({ minOpenedCount: 10 }, 'minAgeDays')).toBeNull(); + }); + + it('prefers the retention direction when a hand-edited link carries both', () => { + expect(retentionEntryValue({ maxSeenCount: 5, minSeenCount: 2 }, 'maxSeenCount')).toEqual({ + field: 'maxSeenCount', + value: 5, + }); + }); +}); + +describe('retentionValueRows — reverse directions', () => { + it('offers the reverse direction its own presets, and a typed value in that direction', () => { + expect(retentionValueRows('minOpenedCount', '').map((r) => r.value)).toEqual([1, 5, 10, 25]); + // Typed values are the custom-value input in BOTH directions. + expect(retentionValueRows('minOpenedCount', '40')).toEqual([{ value: 40, custom: true }]); + expect(retentionValueRows('openedWithinDays', '14')).toEqual([{ value: 14, custom: true }]); + }); + + it('offers no typed value that narrows nothing', () => { + expect(retentionValueRows('minOpenedCount', '0')).toEqual([]); + // Every memory was written at least once (a table constraint), so a + // recurrence floor of 1 is the same no-op as a count floor of 0. + // (The preset 10 still matches by numeric prefix — a reader typing toward + // it — but no row offers 1 itself.) + expect(retentionValueRows('minSeenCount', '1')).toEqual([{ value: 10, custom: false }]); + }); +}); + +describe('parseRetentionQuery', () => { + it('splits an optional comparison from the number', () => { + expect(parseRetentionQuery('')).toBeNull(); + expect(parseRetentionQuery(' ')).toBeNull(); + expect(parseRetentionQuery('50')).toEqual({ op: 'any', digits: '50' }); + expect(parseRetentionQuery('>50')).toEqual({ op: 'gt', digits: '50' }); + expect(parseRetentionQuery('> 50')).toEqual({ op: 'gt', digits: '50' }); + expect(parseRetentionQuery('<50')).toEqual({ op: 'lt', digits: '50' }); + // Two-character operators are read whole, never as `>` then `=50`. + expect(parseRetentionQuery('>=50')).toEqual({ op: 'gte', digits: '50' }); + expect(parseRetentionQuery('<=50')).toEqual({ op: 'lte', digits: '50' }); + expect(parseRetentionQuery('≥50')).toEqual({ op: 'gte', digits: '50' }); + expect(parseRetentionQuery('≤50')).toEqual({ op: 'lte', digits: '50' }); + expect(parseRetentionQuery('>')).toEqual({ op: 'gt', digits: '' }); + }); +}); + +describe('retentionEntryRows — both directions in one list', () => { + const rows = (entry: Parameters[0], query: string, conditions = {}) => + retentionEntryRows(entry, query, conditions).map((r) => `${r.field}:${r.value}${r.custom ? '*' : ''}`); + + it('lists the retention side\'s presets, then the reverse side\'s, with no query', () => { + expect(rows('maxOpenedCount', '')).toEqual([ + 'maxOpenedCount:0', + 'maxOpenedCount:1', + 'maxOpenedCount:2', + 'maxOpenedCount:5', + 'minOpenedCount:1', + 'minOpenedCount:5', + 'minOpenedCount:10', + 'minOpenedCount:25', + ]); + }); + + it('offers a bare number in every direction it is legal for, then the presets it prefixes', () => { + expect(rows('maxOpenedCount', '40')).toEqual(['maxOpenedCount:40*', 'minOpenedCount:40*']); + // `1` prefixes 10 on the at-least side. + expect(rows('maxOpenedCount', '1')).toEqual(['maxOpenedCount:1', 'minOpenedCount:1', 'minOpenedCount:10']); + // 0 is legal only as a ceiling — a floor of 0 narrows nothing. + expect(rows('maxOpenedCount', '0')).toEqual(['maxOpenedCount:0']); + }); + + it('reads a strict comparison on a COUNT as the next whole number, so the row says what applies', () => { + expect(rows('maxOpenedCount', '>50')).toEqual(['minOpenedCount:51*']); + expect(rows('maxOpenedCount', '<50')).toEqual(['maxOpenedCount:49*']); + expect(rows('maxOpenedCount', '>=50')).toEqual(['minOpenedCount:50*']); + expect(rows('maxOpenedCount', '<=5')).toEqual(['maxOpenedCount:5']); + // `<1` is "never chosen"; `>0` is "at least once". + expect(rows('maxOpenedCount', '<1')).toEqual(['maxOpenedCount:0']); + expect(rows('maxOpenedCount', '>0')).toEqual(['minOpenedCount:1']); + }); + + it('leaves DAYS unshifted — time is continuous, so >30 is "more than 30 days ago" as is', () => { + expect(rows('unseenDays', '>30')).toEqual(['unseenDays:30']); + expect(rows('unseenDays', '<7')).toEqual(['openedWithinDays:7']); + expect(rows('unseenDays', '14')).toEqual(['unseenDays:14*', 'openedWithinDays:14*']); + }); + + it('narrows to one side\'s presets on a bare operator', () => { + expect(rows('maxReadCount', '>')).toEqual([ + 'minReadCount:100', + 'minReadCount:200', + 'minReadCount:500', + 'minReadCount:1000', + ]); + }); + + it('offers nothing for a direction the row cannot read, or a value out of bounds', () => { + // Created reads one way; "created within" is the date range's job. + expect(rows('minAgeDays', '<30')).toEqual([]); + expect(rows('minAgeDays', '>30')).toEqual(['minAgeDays:30']); + expect(rows('maxOpenedCount', '<0')).toEqual([]); + expect(rows('maxOpenedCount', 'abc')).toEqual([]); + }); + + it('leads each group with its applied custom value, so the list never denies the pill', () => { + expect(rows('maxOpenedCount', '', { minOpenedCount: 40 }).slice(4, 6)).toEqual([ + 'minOpenedCount:40*', + 'minOpenedCount:1', + ]); + }); +}); + describe('setRetentionCondition', () => { + it('clears the same row\'s opposite direction when setting a value', () => { + // One direction per row: switching `Chosen` from "at most 5" to "at least + // 10" must REPLACE the condition, not narrow to the empty range 10..5. + expect(setRetentionCondition({ maxOpenedCount: 5, minAgeDays: 30 }, 'minOpenedCount', 10)).toEqual({ + minAgeDays: 30, + minOpenedCount: 10, + }); + expect(setRetentionCondition({ openedWithinDays: 7 }, 'unseenDays', 90)).toEqual({ unseenDays: 90 }); + }); + + it('leaves the opposite alone when CLEARING, so a pill × removes only its own pill', () => { + expect(setRetentionCondition({ maxSeenCount: 5, minSeenCount: 2 }, 'minSeenCount', undefined)).toEqual({ + maxSeenCount: 5, + }); + }); + it('sets a field without mutating the input', () => { const before: RetentionConditions = { minAgeDays: 30 }; const after = setRetentionCondition(before, 'maxOpenedCount', 0); @@ -595,6 +853,10 @@ describe('policyRulePhrase', () => { policyRulePhrase({}, [{ field: 'host', operator: 'in', values: ['reviewer'] }]), ).toContain('reviewer'); }); + + it('names only what a policy can carry, never a reverse direction', () => { + expect(policyRulePhrase({ maxOpenedCount: 0, minReadCount: 100 })).toBe('chosen ≤ 0×'); + }); }); describe('policyNameFromRule', () => { diff --git a/packages/web/src/lib/retention-filter.ts b/packages/web/src/lib/retention-filter.ts index ae7e42f76..6b58b5772 100644 --- a/packages/web/src/lib/retention-filter.ts +++ b/packages/web/src/lib/retention-filter.ts @@ -23,6 +23,30 @@ * Kept dependency-free and pure, in `lib/filters.ts`'s tradition — every * decision here is unit-tested without a browser, and the impure shells are * `components/lore/FilterMenu.tsx` and `components/lore/RetentionPill.tsx`. + * + * ## Both directions (migration 00113) + * + * The five conditions above all point the retention way — "N or fewer", "not + * opened for N days" — which is what a groomer asks before archiving, and + * nothing else. Four more read the same metadata rows the OTHER way, for the + * opposite question, "what is actually used?": + * + * Last agent open unseenDays (more than N days ago) ↔ openedWithinDays + * Recurrence maxSeenCount ↔ minSeenCount + * Delivered maxReadCount ↔ minReadCount + * Chosen maxOpenedCount ↔ minOpenedCount + * + * Each pair shares ONE menu row and ONE label, and the row's value list holds + * BOTH directions as ordinary rows — the retention side's presets, then the + * reverse side's — so nothing is hidden behind a mode. A typed number offers + * both directions, and `>50` / `<50` (also `>=`, `<=`, `≥`, `≤`) pick one (see + * {@link retentionEntryRows}). Picking a value in one direction clears the other + * (see {@link setRetentionCondition}). The four reverse directions are EXPLORER-ONLY: + * a retention policy cannot carry them (`GroomConditionsSchema` deliberately + * lacks them), so {@link retentionConditionsToGroomConditions} drops them and + * {@link hasExplorerOnlyConditions} is what hides "Create retention policy" + * while one is set — a policy saved from that view would archive a different + * set from the one on screen. */ import type { GroomConditions, GroomDimensionFilters } from '@lorekit/schemas/retention'; @@ -61,8 +85,36 @@ export interface RetentionConditions { * migration header for the measured dead zone that motivated it. */ maxOpenedCount?: number; + /** + * Last deliberately fetched within N days — `opened_within_days` (00113), the + * reverse of `unseenDays`. A lesson NEVER opened does not match, however new: + * "opened recently" is a claim about an agent that no agent made. + * Explorer-only, like the three floors below. + */ + openedWithinDays?: number; + /** Written at least N times — `min_seen_count` (00113), the reverse of `maxSeenCount`. */ + minSeenCount?: number; + /** Delivered at least N times — `min_read_count` (00113), the reverse of `maxReadCount`. */ + minReadCount?: number; + /** + * Deliberately fetched at least N times — `min_opened_count` (00113), the + * reverse of `maxOpenedCount`: the lore agents actually reach for. + */ + minOpenedCount?: number; } +/** + * The five conditions a saved retention policy can carry — `GroomConditions`' + * thresholds, camelCased. The other four keys of {@link RetentionConditions} + * are the Explorer's reverse directions and never reach a policy. + */ +export type PolicyRetentionField = + | 'minAgeDays' + | 'unseenDays' + | 'maxSeenCount' + | 'maxReadCount' + | 'maxOpenedCount'; + /** * The bounds each condition accepts — mirrors `GroomConditionsSchema` exactly. * Exported so the impure shells (the filter menu's typed custom value, the @@ -76,7 +128,26 @@ export const RETENTION_CONDITION_BOUNDS = { maxSeenCount: { min: 0, max: 100_000 }, maxReadCount: { min: 0, max: 100_000 }, maxOpenedCount: { min: 0, max: 100_000 }, -} as const; + // The reverse directions start at the API's own bound — the first value that + // can narrow anything. A floor of 0 matches every row, and so does a + // recurrence floor of 1: `seen_count >= 1` is a table constraint (00059). + openedWithinDays: { min: 1, max: 3650 }, + minSeenCount: { min: 2, max: 100_000 }, + minReadCount: { min: 1, max: 100_000 }, + minOpenedCount: { min: 1, max: 100_000 }, +} as const satisfies Record; + +/** Every condition key, in the bounds table's order. */ +const CONDITION_FIELDS = Object.keys(RETENTION_CONDITION_BOUNDS) as (keyof RetentionConditions)[]; + +/** The five conditions a retention policy can carry, in the menu's order. */ +export const POLICY_RETENTION_FIELDS: readonly PolicyRetentionField[] = [ + 'minAgeDays', + 'unseenDays', + 'maxSeenCount', + 'maxReadCount', + 'maxOpenedCount', +]; /** An empty condition set — nothing narrowed. Module-scoped for reference stability. */ export const NO_RETENTION_CONDITIONS: RetentionConditions = {}; @@ -120,7 +191,7 @@ export const RETENTION_CONDITION_PLACEHOLDERS = { * active, and the resulting list looked like the filter was broken. `e.g. ` * cannot be misread as a value, and still shows the reader what to type. */ -export function retentionConditionPlaceholder(field: keyof RetentionConditions): string { +export function retentionConditionPlaceholder(field: PolicyRetentionField): string { return `e.g. ${RETENTION_CONDITION_PLACEHOLDERS[field]}`; } @@ -143,45 +214,41 @@ export function parseCondition(raw: unknown, bounds: { min: number; max: number */ export function normalizeRetentionConditions(raw: unknown): RetentionConditions { if (!raw || typeof raw !== 'object') return {}; - const { minAgeDays, unseenDays, maxSeenCount, maxReadCount, maxOpenedCount } = raw as Record< - string, - unknown - >; + const source = raw as Record; + // Walked off the bounds table, so a condition added there is parsed here + // without a second list to keep in step — and an unknown key in a + // hand-edited link is simply never read. const out: RetentionConditions = {}; - const min = parseCondition(minAgeDays, RETENTION_CONDITION_BOUNDS.minAgeDays); - if (min !== undefined) out.minAgeDays = min; - const unseen = parseCondition(unseenDays, RETENTION_CONDITION_BOUNDS.unseenDays); - if (unseen !== undefined) out.unseenDays = unseen; - const maxSeen = parseCondition(maxSeenCount, RETENTION_CONDITION_BOUNDS.maxSeenCount); - if (maxSeen !== undefined) out.maxSeenCount = maxSeen; - const maxRead = parseCondition(maxReadCount, RETENTION_CONDITION_BOUNDS.maxReadCount); - if (maxRead !== undefined) out.maxReadCount = maxRead; - const maxOpened = parseCondition(maxOpenedCount, RETENTION_CONDITION_BOUNDS.maxOpenedCount); - if (maxOpened !== undefined) out.maxOpenedCount = maxOpened; + for (const field of CONDITION_FIELDS) { + const value = parseCondition(source[field], RETENTION_CONDITION_BOUNDS[field]); + if (value !== undefined) out[field] = value; + } return out; } -/** Whether any condition is set — what gates the "Create retention policy" action. */ +/** Whether any condition is set, in either direction. */ export function hasRetentionConditions(conditions: RetentionConditions): boolean { - return ( - conditions.minAgeDays !== undefined || - conditions.unseenDays !== undefined || - conditions.maxSeenCount !== undefined || - conditions.maxReadCount !== undefined || - conditions.maxOpenedCount !== undefined - ); + return CONDITION_FIELDS.some((field) => conditions[field] !== undefined); } /** How many conditions are set — the control's count badge. */ export function retentionConditionsCount(conditions: RetentionConditions): number { - return [ - conditions.minAgeDays, - conditions.unseenDays, - conditions.maxSeenCount, - conditions.maxReadCount, - conditions.maxOpenedCount, - ].filter((v) => v !== undefined).length; + return CONDITION_FIELDS.filter((field) => conditions[field] !== undefined).length; +} + +/** + * Whether a condition a retention policy cannot carry is set — one of the four + * reverse directions. What hides "Create retention policy": a policy saved from + * this view would match a different set from the one on screen. + */ +export function hasExplorerOnlyConditions(conditions: RetentionConditions): boolean { + return ( + conditions.openedWithinDays !== undefined || + conditions.minSeenCount !== undefined || + conditions.minReadCount !== undefined || + conditions.minOpenedCount !== undefined + ); } /** @@ -202,19 +269,34 @@ export function retentionConditionsParamValue( */ export function retentionConditionsToListBody( conditions: RetentionConditions, -): Pick< - ListMemoriesBody, - 'min_age_days' | 'unseen_days' | 'max_seen_count' | 'max_read_count' | 'max_opened_count' -> { +): Pick { return { ...(conditions.minAgeDays !== undefined ? { min_age_days: conditions.minAgeDays } : {}), ...(conditions.unseenDays !== undefined ? { unseen_days: conditions.unseenDays } : {}), ...(conditions.maxSeenCount !== undefined ? { max_seen_count: conditions.maxSeenCount } : {}), ...(conditions.maxReadCount !== undefined ? { max_read_count: conditions.maxReadCount } : {}), ...(conditions.maxOpenedCount !== undefined ? { max_opened_count: conditions.maxOpenedCount } : {}), + ...(conditions.openedWithinDays !== undefined + ? { opened_within_days: conditions.openedWithinDays } + : {}), + ...(conditions.minSeenCount !== undefined ? { min_seen_count: conditions.minSeenCount } : {}), + ...(conditions.minReadCount !== undefined ? { min_read_count: conditions.minReadCount } : {}), + ...(conditions.minOpenedCount !== undefined ? { min_opened_count: conditions.minOpenedCount } : {}), }; } +/** The wire names of all nine conditions, shared by the list and aggregate bodies. */ +type ConditionBodyField = + | 'min_age_days' + | 'unseen_days' + | 'max_seen_count' + | 'max_read_count' + | 'max_opened_count' + | 'opened_within_days' + | 'min_seen_count' + | 'min_read_count' + | 'min_opened_count'; + /** * The same five fields, for the three AGGREGATE routes — `/facets`, * `/activity`, `/pivot` (migration 00108). @@ -233,10 +315,7 @@ export function retentionConditionsToListBody( */ export function retentionConditionsToAggregateBody( conditions: RetentionConditions, -): Pick< - ListFacetsBody, - 'min_age_days' | 'unseen_days' | 'max_seen_count' | 'max_read_count' | 'max_opened_count' -> { +): Pick { return retentionConditionsToListBody(conditions); } @@ -244,13 +323,29 @@ export function retentionConditionsToAggregateBody( * Translate into the `GroomConditions` a policy carries — the seam this whole * feature exists for: "create a retention policy from these filters" is * exactly handing this struct (plus a scope) to `policy.create`. + * + * Only the five policy-expressible conditions cross. The reverse directions + * (00113) are dropped rather than forwarded, because `GroomConditionsSchema` + * has no field for them — and a policy is the one place a condition must never + * silently mean something other than what the reader set. */ export function retentionConditionsToGroomConditions( conditions: RetentionConditions, ): GroomConditions { - return retentionConditionsToListBody(conditions); + return retentionConditionsToListBody(policyRetentionConditions(conditions)); } +/** Just the five conditions a retention policy can carry. */ +export function policyRetentionConditions(conditions: RetentionConditions): RetentionConditions { + const out: RetentionConditions = {}; + for (const field of POLICY_RETENTION_FIELDS) { + const value = conditions[field]; + if (value !== undefined) out[field] = value; + } + return out; +} + + // ── Dimension filters ──────────────────────────────────────────────────────── // // A retention policy can ALSO carry the same eight dimension filters the @@ -390,7 +485,10 @@ export function groomConditionsToFilters(source: DimensionFilterSource): Filter[ // input (see `retentionValueRows`). Typing `45` offers 45; there is no "Custom…" // row, no third level, and no second text field. -/** Which of the five conditions a menu entry addresses. */ +/** + * Which condition something addresses — any of the nine, in either direction. + * A menu ROW is narrower: it is always a {@link PolicyRetentionField}. + */ export type RetentionField = keyof RetentionConditions; /** One age/activity entry: its row at level 1, and how its values read at level 2. */ @@ -417,25 +515,66 @@ export interface RetentionFieldDescriptor { formatValue: (value: number) => string; /** One value as a phrase fragment, for the pill and the summary line. */ phrase: (value: number) => string; + /** + * The menu row this condition lives under — its own field for the five rows + * the menu lists, the row's field for a reverse direction. Both directions of + * one metadata row share a label and a row, so a reader finds "Chosen" in one + * place whichever way they want to read it. + */ + entry: PolicyRetentionField; + /** The same metadata row read the other way, when one exists (00113). */ + opposite?: RetentionField; + /** This direction's group heading in its row's value list — `At most`, `Within`. */ + directionLabel?: string; + /** + * Which side of N this direction keeps, on the row's own quantity (a count, + * or days since the event): `upper` keeps N and below (`5 times or fewer`, + * `Within 7 days`), `lower` keeps N and above (`10 times or more`, `More than + * 30 days ago`). It is what a typed `<` or `>` routes to. + */ + bound: 'upper' | 'lower'; + /** + * Whether the quantity is a whole count. A strict `>50` on a count means 51 + * and up, so it is stored as 51; on days it stays 50, because time is + * continuous and "more than 50 days ago" is exactly what 50 already tests. + */ + discrete: boolean; + /** The menu row's hint when its list holds BOTH directions — covers either reading. */ + entryHint?: string; + /** A representative number for the menu row's search placeholder (`type 10, >10 or <10`). */ + typeExample?: number; } const plural = (n: number, noun: string) => `${n} ${noun}${n === 1 ? '' : 's'}`; +/** `1 time or more` reads as a hedge; a single fetch is "at least once". */ +const atLeastTimes = (n: number) => (n === 1 ? 'At least once' : `${plural(n, 'time')} or more`); + /** - * The five entries, in the order the menu lists them: the two dates first, then - * the three counters weakest-signal-first, so `Chosen` — the one whose `0` - * answers "nothing ever used this" — sits closest to the reader's eye at the - * end rather than buried between two counters it is easily confused with. + * Every condition, in display order — each menu row's retention direction + * followed by its reverse (00113), so pills and summary phrases read in the + * menu's own order. The menu lists only the five rows ({@link RETENTION_MENU_FIELDS}): + * the two dates first, then the three counters weakest-signal-first, so + * `Chosen` — the one whose `0` answers "nothing ever used this" — sits closest + * to the reader's eye at the end rather than buried between two counters it is + * easily confused with. * * The three counters are worded APART on purpose, because each answers a * different question: `Recurrence` counts WRITES, `Delivered` counts EVERY read * (bulk list/search ride-alongs included), and `Chosen` counts only deliberate * `memory.read` fetches. Calling any of them "seen" is what made the older copy * unreadable. + * + * A reverse direction keeps its row's LABEL, so a pill reads `Chosen: 10 times + * or more` beside `Chosen: 5 times or fewer` — the value's own wording carries + * the direction, exactly as it always has for the retention side. */ export const RETENTION_FIELDS: readonly RetentionFieldDescriptor[] = [ { field: 'minAgeDays', + entry: 'minAgeDays', + bound: 'lower', + discrete: false, label: 'Created', hint: 'Written more than this many days ago.', presets: [7, 30, 90, 180, 365], @@ -445,18 +584,48 @@ export const RETENTION_FIELDS: readonly RetentionFieldDescriptor[] = [ }, { field: 'unseenDays', + entry: 'unseenDays', + bound: 'lower', + discrete: false, + entryHint: 'When an agent last deliberately fetched it. More than: a lesson never opened counts from Created. Within: a lesson never opened does not match.', + typeExample: 30, + opposite: 'openedWithinDays', + directionLabel: 'More than', label: 'Last agent open', // Spelling out the never-opened fallback is the point: it is the one rule a // reader cannot infer from a lesson's metadata, and getting it wrong is // what made a week-old lesson look like it had gone unread for 90 days. hint: 'Last deliberately fetched more than this many days ago. Never opened counts from Created.', presets: [7, 30, 90, 180, 365], - keywords: ['unseen', 'unopened', 'stale', 'idle', 'dormant', 'last'], + keywords: ['unseen', 'unopened', 'stale', 'idle', 'dormant', 'last', 'recent', 'recently', 'active'], formatValue: (n) => `More than ${plural(n, 'day')} ago`, phrase: (n) => `unopened >${n}d`, }, + { + field: 'openedWithinDays', + entry: 'unseenDays', + bound: 'upper', + discrete: false, + opposite: 'unseenDays', + directionLabel: 'Within', + label: 'Last agent open', + // The never-opened rule is the OPPOSITE of the retention side's, and saying + // so is what stops "Within 7 days" reading as "new lore". + hint: 'Last deliberately fetched within this many days. A lesson never opened does not match.', + presets: [1, 7, 30, 90], + keywords: [], + formatValue: (n) => `Within ${plural(n, 'day')}`, + phrase: (n) => `opened within ${n}d`, + }, { field: 'maxSeenCount', + entry: 'maxSeenCount', + bound: 'upper', + discrete: true, + entryHint: 'How many times it was written. Many writes means agents keep relearning it.', + typeExample: 3, + opposite: 'minSeenCount', + directionLabel: 'At most', label: 'Recurrence', hint: 'Written this many times or fewer.', // From 1, not 0: a memory exists because it was written, so `0` matches @@ -466,8 +635,29 @@ export const RETENTION_FIELDS: readonly RetentionFieldDescriptor[] = [ formatValue: (n) => `${plural(n, 'time')} or fewer`, phrase: (n) => `written ≤ ${n}×`, }, + { + field: 'minSeenCount', + entry: 'maxSeenCount', + bound: 'lower', + discrete: true, + opposite: 'maxSeenCount', + directionLabel: 'At least', + label: 'Recurrence', + hint: 'Written this many times or more — the lessons agents keep relearning.', + presets: [2, 3, 5, 10], + keywords: [], + formatValue: atLeastTimes, + phrase: (n) => `written ≥ ${n}×`, + }, { field: 'maxReadCount', + entry: 'maxReadCount', + bound: 'upper', + discrete: true, + entryHint: 'How many times it was handed to an agent. Bulk list/search reads count, so this runs to the hundreds.', + typeExample: 200, + opposite: 'minReadCount', + directionLabel: 'At most', label: 'Delivered', hint: 'Handed to an agent this many times or fewer. Bulk list/search reads count, so this runs to the hundreds.', // Measured against the live store: `max_read_count <= 5` matched nothing at @@ -479,19 +669,64 @@ export const RETENTION_FIELDS: readonly RetentionFieldDescriptor[] = [ formatValue: (n) => `${plural(n, 'time')} or fewer`, phrase: (n) => `delivered ≤ ${n}×`, }, + { + field: 'minReadCount', + entry: 'maxReadCount', + bound: 'lower', + discrete: true, + opposite: 'maxReadCount', + directionLabel: 'At least', + label: 'Delivered', + hint: 'Handed to an agent this many times or more. Bulk list/search reads count, so this runs to the hundreds.', + // The same measurement read the other way: the interesting floor is in the + // hundreds, and 1000 is where only the broadest-reaching lore remains. + presets: [100, 200, 500, 1000], + keywords: [], + formatValue: atLeastTimes, + phrase: (n) => `delivered ≥ ${n}×`, + }, { field: 'maxOpenedCount', + entry: 'maxOpenedCount', + bound: 'upper', + discrete: true, + entryHint: 'How many times an agent deliberately fetched it. Bulk reads do NOT count, so 0 means never chosen.', + typeExample: 10, + opposite: 'minOpenedCount', + directionLabel: 'At most', label: 'Chosen', hint: 'Deliberately fetched this many times or fewer. Bulk reads do NOT count, so 0 means never chosen.', // 0 leads, and is the only preset in the set for which zero is meaningful: // it is migration 00105's whole point, "nothing ever chose this lesson". presets: [0, 1, 2, 5], - keywords: ['chosen', 'opened', 'never', 'unused', 'uptake', 'dead'], + keywords: ['chosen', 'opened', 'never', 'unused', 'uptake', 'dead', 'most', 'popular', 'top', 'used'], formatValue: (n) => (n === 0 ? 'Never chosen' : `${plural(n, 'time')} or fewer`), phrase: (n) => `chosen ≤ ${n}×`, }, + { + field: 'minOpenedCount', + entry: 'maxOpenedCount', + bound: 'lower', + discrete: true, + opposite: 'maxOpenedCount', + directionLabel: 'At least', + label: 'Chosen', + hint: 'Deliberately fetched this many times or more — the lore agents actually reach for. Bulk reads do NOT count.', + presets: [1, 5, 10, 25], + keywords: [], + formatValue: atLeastTimes, + phrase: (n) => `chosen ≥ ${n}×`, + }, ]; +/** + * The five menu rows, in menu order — one per metadata row, whichever + * direction is applied. Each row's own field is its retention direction. + */ +export const RETENTION_MENU_FIELDS: readonly PolicyRetentionField[] = RETENTION_FIELDS.flatMap((d) => + d.field === d.entry ? [d.entry] : [], +); + const RETENTION_FIELD_BY_NAME = new Map(RETENTION_FIELDS.map((d) => [d.field, d])); /** @@ -515,9 +750,10 @@ export function requireRetentionField(field: RetentionField): RetentionFieldDesc * `Created` and `Chosen` rather than with whatever keyword happens to start * with the same letter. */ -export function retentionFieldMatches(query: string): RetentionField[] { +export function retentionFieldMatches(query: string): PolicyRetentionField[] { + const menu = RETENTION_MENU_FIELDS.map(requireRetentionField); const needle = query.trim().toLowerCase(); - if (!needle) return RETENTION_FIELDS.map((d) => d.field); + if (!needle) return [...RETENTION_MENU_FIELDS]; const rank = (d: RetentionFieldDescriptor): number => { const label = d.label.toLowerCase(); @@ -528,10 +764,32 @@ export function retentionFieldMatches(query: string): RetentionField[] { return Number.POSITIVE_INFINITY; }; - return RETENTION_FIELDS.map((d) => ({ d, rank: rank(d) })) + return menu + .map((d) => ({ d, rank: rank(d) })) .filter(({ rank: r }) => Number.isFinite(r)) .sort((a, b) => a.rank - b.rank) - .map(({ d }) => d.field); + .map(({ d }) => d.entry); +} + +/** + * The condition a menu row currently holds, in whichever direction — or `null` + * when the row narrows nothing. The retention direction wins when a hand-edited + * link carries both, since that is the one the row is named for; both still + * apply and both still render as pills. + * + * This is what the row's badge shows and which direction its value list opens + * on, so reopening `Chosen` while `Chosen: 10 times or more` is applied lands + * on the `At least` list with 10 selected, not on an `At most` list denying it. + */ +export function retentionEntryValue( + conditions: RetentionConditions, + entry: PolicyRetentionField, +): { field: RetentionField; value: number } | null { + const own = conditions[entry]; + if (own !== undefined) return { field: entry, value: own }; + const { opposite } = requireRetentionField(entry); + const reverse = opposite === undefined ? undefined : conditions[opposite]; + return opposite !== undefined && reverse !== undefined ? { field: opposite, value: reverse } : null; } /** One row in a condition's value list. */ @@ -599,10 +857,119 @@ export function retentionValueRows( ]; } +/** What a value-list query asks for: an optional comparison, then the number text. */ +export interface RetentionQuery { + /** `any` for a bare number (both directions); otherwise the side it picks. */ + op: 'any' | 'gt' | 'gte' | 'lt' | 'lte'; + /** Everything after the operator, trimmed — matched against presets by prefix. */ + digits: string; +} + +const QUERY_OPERATOR: Record = { + '>': 'gt', + '>=': 'gte', + '≥': 'gte', + '<': 'lt', + '<=': 'lte', + '≤': 'lte', +}; + +/** + * Split a value-list query into its comparison and its number. `null` for an + * empty query. `>=` is tried before `>` so it is never read as `>` then `=50`. + */ +export function parseRetentionQuery(query: string): RetentionQuery | null { + const trimmed = query.trim(); + if (!trimmed) return null; + const match = /^(>=|<=|≥|≤|>|<)?\s*(.*)$/.exec(trimmed); + const operator = match?.[1]; + return { + op: operator ? (QUERY_OPERATOR[operator] ?? 'any') : 'any', + digits: (match?.[2] ?? '').trim(), + }; +} + +/** One row of a menu row's value list: which direction it applies, and the value. */ +export interface RetentionEntryRow extends RetentionValueRow { + field: RetentionField; +} + +/** + * The value a typed query applies in one direction, or `undefined` when the + * query does not ask for that direction or lands out of bounds. + * + * A strict comparison on a COUNT moves by one — `>50` keeps 51 and up, so it is + * stored and shown as `51 times or more` — because the stored conditions are + * inclusive and the row must read exactly as the filter it applies. On days it + * does not move: `>30` under Last agent open is `More than 30 days ago` as is. + */ +function typedValue(d: RetentionFieldDescriptor, q: RetentionQuery): number | undefined { + if (q.op === 'gt' || q.op === 'gte') { + if (d.bound !== 'lower') return undefined; + } else if (q.op === 'lt' || q.op === 'lte') { + if (d.bound !== 'upper') return undefined; + } + const n = parseCondition(q.digits, { min: Number.MIN_SAFE_INTEGER, max: Number.MAX_SAFE_INTEGER }); + if (n === undefined) return undefined; + const shift = !d.discrete ? 0 : q.op === 'gt' ? 1 : q.op === 'lt' ? -1 : 0; + return parseCondition(n + shift, RETENTION_CONDITION_BOUNDS[d.field]); +} + +/** + * A menu row's whole value list: its retention direction's rows, then its + * reverse direction's — one list, so both readings are visible at once and + * neither hides behind a mode. + * + * - No query: each direction's presets, with an applied custom value leading + * its own group (see {@link retentionValueRows}). + * - A bare number: that value in EVERY direction it is legal for, each + * followed by the presets it prefixes — typing `40` under Chosen offers both + * `40 times or fewer` and `40 times or more`. + * - `>N` / `=`, `<=`, `≥`, `≤`): the one direction that comparison + * names, as a single row. An operator with no number yet (`>`) narrows the + * list to that direction's presets, so the reader sees what the side holds. + * + * A comparison the row cannot read — `<30` under Created, which has no reverse + * — yields nothing, and the menu's empty state says why. + */ +export function retentionEntryRows( + entry: PolicyRetentionField, + query: string, + conditions: RetentionConditions, +): RetentionEntryRow[] { + const own = requireRetentionField(entry); + const directions = own.opposite === undefined ? [own] : [own, requireRetentionField(own.opposite)]; + const q = parseRetentionQuery(query); + + return directions.flatMap((d): RetentionEntryRow[] => { + const tag = (rows: RetentionValueRow[]) => rows.map((row) => ({ ...row, field: d.field })); + if (q === null) return tag(retentionValueRows(d.field, '', conditions[d.field])); + + const picksSide = q.op !== 'any'; + const onThisSide = !picksSide || (q.op === 'gt' || q.op === 'gte' ? d.bound === 'lower' : d.bound === 'upper'); + if (!onThisSide) return []; + if (picksSide && q.digits === '') return tag(retentionValueRows(d.field, '', conditions[d.field])); + + const typed = typedValue(d, q); + const typedRows: RetentionValueRow[] = + typed === undefined ? [] : [{ value: typed, custom: !d.presets.includes(typed) }]; + if (picksSide) return tag(typedRows); + const prefixed = d.presets + .filter((preset) => preset !== typed && String(preset).startsWith(q.digits)) + .map((value) => ({ value, custom: false })); + return tag([...typedRows, ...prefixed]); + }); +} + /** * Set or clear one condition — `undefined` removes it, which is how a menu row * toggled off and a pill's × reach the same state. * + * SETTING a value also clears the same row's opposite direction, so a row holds + * one direction at a time: picking `Chosen → At least → 10` after `At most → 5` + * replaces the condition rather than quietly narrowing to the empty range + * "between 10 and 5". Clearing touches only the field named. + * * Returns a new object rather than mutating: the result goes straight into * `?retention=`, and React Query keys the aggregate requests on it. */ @@ -612,8 +979,13 @@ export function setRetentionCondition( value: number | undefined, ): RetentionConditions { const next = { ...conditions }; - if (value === undefined) delete next[field]; - else next[field] = value; + if (value === undefined) { + delete next[field]; + return next; + } + next[field] = value; + const { opposite } = requireRetentionField(field); + if (opposite !== undefined) delete next[opposite]; return next; } @@ -675,8 +1047,11 @@ export function policyRulePhrase( conditions: RetentionConditions, filters: readonly Filter[] = [], ): string { + // Through `policyRetentionConditions`, so a policy's name can only ever + // describe conditions a policy can carry. + const policy = policyRetentionConditions(conditions); const parts = RETENTION_FIELDS.flatMap(({ field, phrase }) => { - const value = conditions[field]; + const value = policy[field]; return value === undefined ? [] : [phrase(value)]; }); // `filtersPhrase` answers 'Add filter' for an empty bar — a button label, not diff --git a/supabase/functions/_shared/api/count-cursor.ts b/supabase/functions/_shared/api/count-cursor.ts new file mode 100644 index 000000000..8c314b4d0 --- /dev/null +++ b/supabase/functions/_shared/api/count-cursor.ts @@ -0,0 +1,78 @@ +/** + * The keyset cursor for the `sort=opened_count` list — "most chosen first" + * (migration 00113). + * + * Pure and shared (mirrored into `supabase/functions/_shared/api/count-cursor.ts`, + * guarded by `edge-parity.spec.ts`) so the codec is unit-tested here and the + * deployed copy cannot drift from what was tested. + * + * Why it is not one more case in `_shared/api/paginate.ts`: that cursor carries + * a TIMESTAMP — both date sorts put their last row's value in it and the list + * RPC compares it as `timestamptz` — and `paginate.ts` is byte-mirrored into the + * MCP function, which has no count sort. An integer bound needs an integer + * field, so this is its own codec with its own shape: `{ id, opened_count }`. + * + * The two formats cannot be mistaken for each other. `paginate.ts`'s decoder + * finds neither `updated_at` nor `created_at` in a count cursor and drops it, + * and this decoder finds no integer `opened_count` in a date cursor and drops + * that. A caller who flips `sort` mid-pagination therefore restarts at page one + * rather than receiving a page cut at the wrong place. + */ + +/** The decoded keyset position: the last row's id and its `opened_count`. */ +export interface CountCursor { + id: string; + count: number; +} + +/** A cursor's `id` is a `memories` primary key, which is a `uuid`. */ +const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +/** `opened_count` is a Postgres `integer`, so a bound past int4 cannot be a real row's value. */ +const INT4_MAX = 2_147_483_647; + +/** base64url without padding — the same alphabet `paginate.ts` uses. */ +export function encodeCountCursor(id: string, count: number): string { + return btoa(JSON.stringify({ id, opened_count: count })) + .replace(/\+/g, '-') + .replace(/\//g, '_') + .replace(/=/g, ''); +} + +/** + * Decode a count cursor, or `null` for anything that is not one. + * + * Fails CLOSED: the values reach the RPC as typed parameters (`uuid`, + * `integer`), so a malformed one would be a 500 rather than an injection — and + * the right answer to a cursor that does not parse is the first page, the same + * contract `paginate.ts` keeps. + */ +export function decodeCountCursor(raw: string): CountCursor | null { + try { + const b64 = raw.replace(/-/g, '+').replace(/_/g, '/'); + const pad = (4 - (b64.length % 4)) % 4; + const parsed: unknown = JSON.parse(atob(b64 + '='.repeat(pad))); + if (!parsed || typeof parsed !== 'object') return null; + const { id, opened_count: count } = parsed as { id?: unknown; opened_count?: unknown }; + if (typeof id !== 'string' || !UUID_RE.test(id)) return null; + if (typeof count !== 'number' || !Number.isInteger(count) || count < 0 || count > INT4_MAX) return null; + return { id, count }; + } catch { + return null; + } +} + +/** + * The page and its next cursor, from `limit + 1` rows — the overflow row is how + * `hasMore` is known, exactly as `paginate.ts`'s `buildPage` reads it. + */ +export function buildCountPage( + rows: T[], + limit: number, +): { entries: T[]; hasMore: boolean; nextCursor: string | null } { + const hasMore = rows.length > limit; + const entries = hasMore ? rows.slice(0, limit) : rows; + const last = entries[entries.length - 1]; + const nextCursor = hasMore && last ? encodeCountCursor(last.id, last.opened_count ?? 0) : null; + return { entries, hasMore, nextCursor }; +} diff --git a/supabase/functions/_shared/api/retention.ts b/supabase/functions/_shared/api/retention.ts index 23632fe34..697b5e24f 100644 --- a/supabase/functions/_shared/api/retention.ts +++ b/supabase/functions/_shared/api/retention.ts @@ -26,6 +26,13 @@ * five field names — the query forms coerce a numeric string, the body forms * require a real number, and by the time either reaches here it is a `number` * or absent. + * + * Migration 00113 added four more, the reverse direction of four of the five: + * `opened_within_days` and the `min_seen_count` / `min_read_count` / + * `min_opened_count` floors. They are READ conditions — the Explorer's "what is + * used the most?" — and `GroomConditionsSchema` deliberately does not carry + * them, but they reach the same four readers through this same mapping for the + * same reason: a count must describe the rows its list returns. */ /** @@ -38,6 +45,10 @@ export interface RetentionConditions { max_seen_count?: number | undefined; max_read_count?: number | undefined; max_opened_count?: number | undefined; + opened_within_days?: number | undefined; + min_seen_count?: number | undefined; + min_read_count?: number | undefined; + min_opened_count?: number | undefined; } /** @@ -56,6 +67,10 @@ export function retentionFrom(input: RetentionConditions): RetentionConditions { max_seen_count: input.max_seen_count, max_read_count: input.max_read_count, max_opened_count: input.max_opened_count, + opened_within_days: input.opened_within_days, + min_seen_count: input.min_seen_count, + min_read_count: input.min_read_count, + min_opened_count: input.min_opened_count, }; } @@ -83,6 +98,10 @@ export function retentionRpcParams(r: RetentionConditions): { p_max_seen_count: number | null; p_max_read_count: number | null; p_max_opened_count: number | null; + p_opened_within_days: number | null; + p_min_seen_count: number | null; + p_min_read_count: number | null; + p_min_opened_count: number | null; } { return { p_min_age_days: r.min_age_days ?? null, @@ -90,5 +109,9 @@ export function retentionRpcParams(r: RetentionConditions): { p_max_seen_count: r.max_seen_count ?? null, p_max_read_count: r.max_read_count ?? null, p_max_opened_count: r.max_opened_count ?? null, + p_opened_within_days: r.opened_within_days ?? null, + p_min_seen_count: r.min_seen_count ?? null, + p_min_read_count: r.min_read_count ?? null, + p_min_opened_count: r.min_opened_count ?? null, }; } diff --git a/supabase/functions/_shared/schemas/memory.ts b/supabase/functions/_shared/schemas/memory.ts index 36ce21891..8e802f654 100644 --- a/supabase/functions/_shared/schemas/memory.ts +++ b/supabase/functions/_shared/schemas/memory.ts @@ -128,8 +128,15 @@ export const TimestampFilterSchema = z 'must be an ISO date (YYYY-MM-DD) or timestamp', ); -/** Sort column for `GET /memories`. The cursor encodes whichever is chosen. */ -export const MemorySortSchema = z.enum(['updated_at', 'created_at']); +/** + * Sort column for `GET /memories`. The cursor encodes whichever is chosen. + * + * `opened_count` (migration 00113) is "most chosen first": descending on the + * deliberate-fetch counter, `id` breaking ties. It pages through a cursor of + * its own (`count-cursor.ts`), because a keyset bound on an integer column + * cannot ride in the timestamp cursor the two date sorts share. + */ +export const MemorySortSchema = z.enum(['updated_at', 'created_at', 'opened_count']); export type MemorySort = z.infer; /** @@ -191,6 +198,28 @@ const ValueListSchema = z.string().min(1).max(2048); * The bounds are copied rather than imported from `retention.ts`: that file is * its own domain file for the reason its header states, and `memory.spec.ts` * pins the two in step. + * + * ## The reverse directions (migration 00113) are READ-ONLY conditions + * + * The five above all point the retention way — "N or fewer", "not opened for + * N days" — because they are what a groomer asks before archiving. The last + * four are their mirrors, for the opposite question ("what is used the most?"): + * + * - `opened_within_days` — last deliberately fetched within N days. A lesson + * NEVER opened does not match, however new: unlike `unseen_days` it does not + * age a never-opened lesson from `created_at`, because "opened recently" is a + * claim about an agent's behaviour that no agent made. + * - `min_seen_count` / `min_read_count` / `min_opened_count` — at least N. + * Each starts at the first value that can narrow anything: 1 for the two + * read counters (a floor of 0 matches every row), and 2 for `min_seen_count`, + * because `seen_count >= 1` is a table constraint (00059) and "written at + * least once" describes every memory there is. + * + * They are deliberately ABSENT from `GroomConditionsSchema`: archiving + * "everything chosen at least 10 times" is not a policy anyone wants, and a + * policy able to express it is one careless edit from archiving the most useful + * lore in the store. So they ride here, on the four read routes, and nowhere a + * write can reach them. */ const retentionConditionBodyFields = { min_age_days: z.number().int().min(1).max(3650).optional(), @@ -198,6 +227,10 @@ const retentionConditionBodyFields = { max_seen_count: z.number().int().min(0).max(100_000).optional(), max_read_count: z.number().int().min(0).max(100_000).optional(), max_opened_count: z.number().int().min(0).max(100_000).optional(), + opened_within_days: z.number().int().min(1).max(3650).optional(), + min_seen_count: z.number().int().min(2).max(100_000).optional(), + min_read_count: z.number().int().min(1).max(100_000).optional(), + min_opened_count: z.number().int().min(1).max(100_000).optional(), } as const; /** @@ -212,6 +245,10 @@ const retentionConditionQueryFields = { max_seen_count: z.coerce.number().int().min(0).max(100_000).optional(), max_read_count: z.coerce.number().int().min(0).max(100_000).optional(), max_opened_count: z.coerce.number().int().min(0).max(100_000).optional(), + opened_within_days: z.coerce.number().int().min(1).max(3650).optional(), + min_seen_count: z.coerce.number().int().min(2).max(100_000).optional(), + min_read_count: z.coerce.number().int().min(1).max(100_000).optional(), + min_opened_count: z.coerce.number().int().min(1).max(100_000).optional(), } as const; export const ListMemoriesQuerySchema = z.object({ diff --git a/supabase/functions/_shared/schemas/openapi/spec.ts b/supabase/functions/_shared/schemas/openapi/spec.ts index d9c43e8aa..e58ccc341 100644 --- a/supabase/functions/_shared/schemas/openapi/spec.ts +++ b/supabase/functions/_shared/schemas/openapi/spec.ts @@ -291,8 +291,9 @@ export function generateSpec(baseUrl = 'https://pqokxlhvnosogizsjztg.supabase.co 'An invalid `scope` is a `400`, not a silently ignored filter — the menu counts and the ' + 'list they drill into agree on what a scope is. The `created_since`/`created_until` window ' + 'and the five retention thresholds (`min_age_days`, `unseen_days`, `max_seen_count`, ' + - '`max_read_count`, `max_opened_count`) narrow the counted population exactly as they do on ' + - '`GET /memories`, so a count matches the list under the same parameters. They emit no facet ' + + '`max_read_count`, `max_opened_count`) and their four reverse directions (`opened_within_days`, ' + + '`min_seen_count`, `min_read_count`, `min_opened_count`) narrow the counted population exactly ' + + 'as they do on `GET /memories`, so a count matches the list under the same parameters. They emit no facet ' + 'rows of their own — they are comparisons, not enumerable dimensions. `q`, `key` and ' + '`expiring_within_days` remain unmirrored, so with those active a count is an upper bound.', security, request: { query: ListFacetsQuerySchema }, @@ -326,8 +327,8 @@ export function generateSpec(baseUrl = 'https://pqokxlhvnosogizsjztg.supabase.co '`row in [x] AND col in [y]` and asks again still gets every other cell back — that ' + 'is what keeps a drilled-in grid navigable instead of collapsing to the clicked cell. ' + 'A pair counting zero emits no cell. `truncated` reports whether `limit` cut the grid. ' + - 'The `created_since`/`created_until` window and the five retention thresholds narrow the ' + - 'counted population, matching `GET /memories/facets`.', + 'The `created_since`/`created_until` window, the five retention thresholds and their four ' + + 'reverse directions narrow the counted population, matching `GET /memories/facets`.', security, request: { query: PivotQuerySchema }, responses: { 200: { description: 'Pivot cells', content: { 'application/json': { schema: PivotResponseSchema } } }, @@ -351,7 +352,8 @@ export function generateSpec(baseUrl = 'https://pqokxlhvnosogizsjztg.supabase.co 'An invalid `scope` is a `400`, not a silently ignored filter — the same rule the read ' + 'counterpart `GET /memories/read-activity` follows. The five retention thresholds ' + '(`min_age_days`, `unseen_days`, `max_seen_count`, `max_read_count`, `max_opened_count`) ' + - 'narrow the counted population exactly as they do on `GET /memories`, so a series matches ' + + 'and their four reverse directions (`opened_within_days`, `min_seen_count`, `min_read_count`, ' + + '`min_opened_count`) narrow the counted population exactly as they do on `GET /memories`, so a series matches ' + 'the list under the same parameters. There is no `created_since`/`created_until` pair here, ' + 'unlike `/facets` and `/pivot`: this route’s own `since`/`until` already bound `created_at`.', security, request: { query: ActivityQuerySchema }, diff --git a/supabase/functions/memories/CLAUDE.md b/supabase/functions/memories/CLAUDE.md index d5b0d9f11..9835d9052 100644 --- a/supabase/functions/memories/CLAUDE.md +++ b/supabase/functions/memories/CLAUDE.md @@ -339,6 +339,19 @@ self-exclusion under OR, and that a cross-group filter still narrows the label c `max_opened_count` narrowed the rows and left every number describing those rows — the Explorer's facet counts, its stat cards and its matrix — counting the un-narrowed population. +**Their four reverse directions ride the same path, as of 00113** — `opened_within_days`, +`min_seen_count`, `min_read_count`, `min_opened_count` — on `GET /`, `POST /list` and all three +aggregates, through the same widened `lorekit_match_retention`. They are READ conditions only: +`GroomConditionsSchema` does not carry them, so no policy can archive "the most chosen lore". +`opened_within_days` deliberately does NOT inherit 00100's never-opened fallback — a lesson +nothing ever opened was not "opened within N days", however new — and each floor starts at the +first value that narrows: 1 for the read counters, 2 for `min_seen_count` (`seen_count >= 1` is a +table constraint). The list also takes `sort=opened_count` (most chosen first, +`id desc` breaking ties); it pages through `_shared/api/count-cursor.ts`'s integer cursor +(mirrored from `mcp-core/src/rest/count-cursor.ts`) rather than `paginate.ts`'s timestamp one, +so `paginate.ts` and its MCP byte-mirror stay untouched, and each decoder rejects the other's +format so a sort flip restarts at page one. §113 pins all of it. + They differ from a dimension in one way that matters when reading a count: a threshold is applied in the RPC's `base` CTE rather than as a per-dimension `ok_*` flag, so it narrows **every** facet including the SELF-EXCLUDED one. That is the correct asymmetry, not an diff --git a/supabase/functions/memories/handlers/list.ts b/supabase/functions/memories/handlers/list.ts index 8e639523b..b523134e2 100644 --- a/supabase/functions/memories/handlers/list.ts +++ b/supabase/functions/memories/handlers/list.ts @@ -5,7 +5,7 @@ import { badRequest, forbidden, ok } from '../../_shared/api/respond.ts'; import { validateOptionalBody, validateQuery } from '../../_shared/api/validate.ts'; import { parseScopeFilter } from '../../_shared/scope/scope.ts'; import { buildPage, decodeCursor } from '../../_shared/api/paginate.ts'; -import type { SortColumn } from '../../_shared/api/paginate.ts'; +import { buildCountPage, decodeCountCursor } from '../../_shared/api/count-cursor.ts'; import { createTracedClient } from '../../_shared/telemetry/otel.ts'; import type { Span } from '../../_shared/telemetry/otel.ts'; import { @@ -13,6 +13,7 @@ import { ListMemoriesQuerySchema, shapeMemoryRow, } from '../../_shared/schemas/memory.ts'; +import type { MemorySort } from '../../_shared/schemas/memory.ts'; import { dimensionsFromBody, dimensionsFromQuery } from '../../_shared/schemas/dimensions.ts'; import type { MemoryDimensions } from '../../_shared/schemas/dimensions.ts'; import { retentionRpcParams } from '../../_shared/api/retention.ts'; @@ -44,7 +45,7 @@ interface ListParams { q?: string | undefined; created_since?: string | undefined; created_until?: string | undefined; - sort: SortColumn; + sort: MemorySort; archived: boolean; expiring_within_days?: number | undefined; /** @@ -58,6 +59,14 @@ interface ListParams { max_seen_count?: number | undefined; max_read_count?: number | undefined; max_opened_count?: number | undefined; + /** + * The reverse directions (00113) — read conditions only, see + * `_shared/api/retention.ts`. + */ + opened_within_days?: number | undefined; + min_seen_count?: number | undefined; + min_read_count?: number | undefined; + min_opened_count?: number | undefined; limit: number; cursor?: string | undefined; dimensions: MemoryDimensions; @@ -172,8 +181,15 @@ async function respondWithPage( // A cursor minted under the other sort order is not comparable with this one, // so it is ignored (→ first page) rather than silently mis-paging. Unchanged // contract; only the place the two halves are applied has moved. - const cursor = params.cursor ? decodeCursor(params.cursor) : null; - const usableCursor = cursor && cursor.sort === params.sort ? cursor : null; + // + // Two cursor FORMATS since 00113: the date sorts keep `paginate.ts`'s + // timestamp cursor, and `opened_count` uses `count-cursor.ts`'s integer one. + // Each decoder rejects the other's format, so the same "wrong sort → first + // page" rule holds across the two families as well as within the dates. + const countSort = params.sort === 'opened_count'; + const dateCursor = !countSort && params.cursor ? decodeCursor(params.cursor) : null; + const usableCursor = dateCursor && dateCursor.sort === params.sort ? dateCursor : null; + const countCursor = countSort && params.cursor ? decodeCountCursor(params.cursor) : null; // The asymmetric `(after, on_or_before]` window stays in TypeScript so the // `now`-relative boundary has exactly one implementation (edge-parity.spec.ts @@ -235,12 +251,15 @@ async function respondWithPage( p_key_org_ids: keyRestriction(auth)?.orgIds ?? [], p_sort: params.sort, p_cursor_ts: usableCursor?.ts ?? null, - p_cursor_id: usableCursor?.id ?? null, + p_cursor_id: usableCursor?.id ?? countCursor?.id ?? null, + // 00113: the count sort's keyset bound. The RPC reads it only when + // `p_sort` is `opened_count`, and `p_cursor_ts` only otherwise. + p_cursor_count: countCursor?.count ?? null, // limit + 1: the overflow row is what `buildPage` reads `hasMore` from. p_limit: params.limit + 1, // The retention thresholds (00092, extended to the aggregates by 00108). // Mapped by the shared `retentionRpcParams` rather than inline, because - // this route is now one of FOUR that take the same five parameters — the + // this route is now one of FOUR that take the same parameters — the // list and the three aggregates describing it — and an inline copy per // route is how a count came to disagree with its own list in the first // place. `?? null` is applied there; see that module on why a truthiness @@ -250,7 +269,10 @@ async function respondWithPage( if (error) { span.error(`DB: ${error.message}`); throw error; } const rows = (data ?? []) as ListRpcRow[]; - const page = buildPage(rows, params.limit, params.sort); + const page = + params.sort === 'opened_count' + ? buildCountPage(rows, params.limit) + : buildPage(rows, params.limit, params.sort); // `total_count` is the SAME value on every row of `rows` (a `count(*) over // ()` — see migration 00094), so any row answers it; an empty result has no // row to read it from, and 0 matches is the exact total in that case too. @@ -312,6 +334,10 @@ export async function handleList( max_seen_count: p.max_seen_count, max_read_count: p.max_read_count, max_opened_count: p.max_opened_count, + opened_within_days: p.opened_within_days, + min_seen_count: p.min_seen_count, + min_read_count: p.min_read_count, + min_opened_count: p.min_opened_count, limit: p.limit, cursor: p.cursor, dimensions: dimensionsFromQuery(p), @@ -356,6 +382,10 @@ export async function handleListPost( max_seen_count: b.max_seen_count, max_read_count: b.max_read_count, max_opened_count: b.max_opened_count, + opened_within_days: b.opened_within_days, + min_seen_count: b.min_seen_count, + min_read_count: b.min_read_count, + min_opened_count: b.min_opened_count, limit: b.limit, cursor: b.cursor, dimensions: dimensionsFromBody(b), diff --git a/supabase/migrations/00113_explorer_reverse_activity_filters_and_opened_sort.sql b/supabase/migrations/00113_explorer_reverse_activity_filters_and_opened_sort.sql new file mode 100644 index 000000000..e66f7b9a0 --- /dev/null +++ b/supabase/migrations/00113_explorer_reverse_activity_filters_and_opened_sort.sql @@ -0,0 +1,1055 @@ +-- ═════════════════════════════════════════════════════════════════════════ +-- 00113 — the Explorer's reverse directions, and a "most chosen" sort. +-- +-- THE GAP. Every age/activity condition the four readers accept points ONE +-- way, because they were built for retention policies: `max_seen_count`, +-- `max_read_count`, `max_opened_count` ("N or fewer"), and `unseen_days` +-- ("not opened for N days"). Those are exactly the questions a groomer asks +-- before archiving. They cannot ask the opposite question — "which lessons are +-- used the MOST?", "what did an agent open this week?" — so the Explorer could +-- show you the dead weight but never the load-bearing lore. +-- +-- THE CHANGE. Four more conditions, each the mirror of one that exists: +-- +-- min_seen_count written at least N times (↔ max_seen_count) +-- min_read_count delivered at least N times (↔ max_read_count) +-- min_opened_count deliberately fetched ≥ N times (↔ max_opened_count) +-- opened_within_days last fetched within N days (↔ unseen_days) +-- +-- plus `p_sort => 'opened_count'` on `lorekit_memory_list`, so "most chosen +-- first" is an ORDER rather than a threshold the reader has to guess. +-- +-- They are READ conditions only. `GroomConditionsSchema` does not gain them and +-- no groom RPC is touched: archiving "everything chosen at least 10 times" is +-- not a retention policy anyone wants, and a policy that could express it would +-- be one careless edit away from archiving the most useful lore in the store. +-- +-- ── DESIGN ─────────────────────────────────────────────────────────────── +-- +-- SAME HELPER, WIDENED. The four new conditions go into +-- `lorekit_match_retention` rather than a second helper, for the reason 00108 +-- created it: every reader composes ONE predicate, so a count cannot disagree +-- with the list it describes (docs/decisions.md, "A COUNT must describe the +-- rows its LIST returns"). Its signature changes, so the 14-argument form is +-- created first, every caller is recreated against it, and the 10-argument +-- form is dropped last. +-- +-- `opened_within_days` IS NOT `unseen_days` NEGATED. `unseen_days` ages a +-- never-opened lesson from `created_at` (00100) so that "unopened for 90 days" +-- can find the lore nothing ever touched. Applying that same fallback here +-- would make a lesson written yesterday and never opened match "opened within +-- 7 days" — a claim about an agent's behaviour that no agent made. So a null +-- `last_opened_at` never satisfies it. +-- +-- THE FLOORS USE `>=` AND START AT THE FIRST VALUE THAT NARROWS at the API +-- (the schemas' bound): 1 for the read counters, since `>= 0` matches every +-- row, and 2 for `min_seen_count`, since `seen_count >= 1` is a table +-- constraint (00059). Accepting a no-op floor would put a pill on screen that +-- narrows nothing. +-- +-- THE COUNT SORT GETS ITS OWN CURSOR PARAMETER. The keyset predicate compares +-- the sort column against the last row's value; the date sorts carry it in +-- `p_cursor_ts timestamptz`, and an integer column needs an integer bound, so +-- `p_cursor_count` is appended and the format() splices in whichever +-- placeholder ($29 or $44) the sort needs. Ties break on `id desc`, exactly as +-- the date sorts do, so a page boundary inside a run of equal counts is stable. +-- +-- ── SIGNATURES ─────────────────────────────────────────────────────────── +-- +-- Appended, null-defaulted parameters on all four readers, so every existing +-- named-argument caller is unaffected. `create or replace` cannot change a +-- signature (00108's header), so each reader is dropped at its EXACT previous +-- argument list first — §108's AC-7 and §113 assert that exactly one overload +-- of each survives. +-- +-- §113 in `migrations.test.sql` is the executable proof. +-- ═════════════════════════════════════════════════════════════════════════ + +-- ── 1. The predicate, widened ─────────────────────────────────────────────── + +-- The five retention thresholds plus the Explorer's four reverse directions, as +-- one row-level test. Every parameter is independently optional (null = not +-- filtered), and all null is a constant `true` the planner discards. +-- +-- `immutable` + `parallel safe` + `language sql` for 00108's reason: the result +-- depends only on the arguments, which is what lets the planner fold the call +-- into an index-usable expression. It still reads no clock — the caller +-- resolves `opened_within_days` to an instant exactly as it does the two +-- existing day counts. +create or replace function lorekit_match_retention( + -- The row's own counters. + p_created_at timestamptz, + p_last_opened_at timestamptz, + p_seen_count integer, + p_read_count integer, + p_opened_count integer, + -- The policy-expressible conditions (00108), day counts already resolved. + p_created_cutoff timestamptz, + p_unseen_cutoff timestamptz, + p_max_seen_count integer, + p_max_read_count integer, + p_max_opened_count integer, + -- 00113: the Explorer's reverse directions. Never carried by a policy. + p_opened_since timestamptz, + p_min_seen_count integer, + p_min_read_count integer, + p_min_opened_count integer +) +returns boolean +language sql +immutable +parallel safe +as $$ + select coalesce( + (p_created_cutoff is null or p_created_at <= p_created_cutoff) + -- 00100: a lesson NEVER opened ages from its creation date, so the lore + -- nothing ever opened is findable by an "unopened for N days" filter. + and (p_unseen_cutoff is null or coalesce(p_last_opened_at, p_created_at) <= p_unseen_cutoff) + -- `<=`, so a threshold of 0 is meaningful (00105). + and (p_max_seen_count is null or p_seen_count <= p_max_seen_count) + and (p_max_read_count is null or p_read_count <= p_max_read_count) + and (p_max_opened_count is null or p_opened_count <= p_max_opened_count) + -- 00113: deliberately WITHOUT 00100's fallback — a never-opened lesson was + -- not "opened within N days", however recently it was written. Spelled out + -- rather than left to the null comparison so the exclusion is visibly the + -- intent and not an accident of three-valued logic. + and (p_opened_since is null + or (p_last_opened_at is not null and p_last_opened_at >= p_opened_since)) + and (p_min_seen_count is null or p_seen_count >= p_min_seen_count) + and (p_min_read_count is null or p_read_count >= p_min_read_count) + and (p_min_opened_count is null or p_opened_count >= p_min_opened_count), + false); +$$; + +comment on function lorekit_match_retention( + timestamptz, timestamptz, integer, integer, integer, + timestamptz, timestamptz, integer, integer, integer, + timestamptz, integer, integer, integer +) is + 'Does a memory satisfy an age/activity condition set? The five thresholds of + GroomConditionsSchema (00108) plus the Explorer''s four reverse directions + (00113: opened-since cutoff, min seen/read/opened counts), as one inlinable + row test shared by lorekit_memory_list, _facets, _activity and _pivot so a + count cannot disagree with the list it describes. Takes CUTOFF INSTANTS + rather than day counts so it stays immutable and reads no clock. A null + argument is "not filtered". Holds 00100''s never-opened rule for unseen + days, and deliberately NOT for opened-since: a never-opened lesson was not + opened recently.'; + +-- The family's ACL (00066/00108): a NEW function starts from EXECUTE-to-public, +-- and Supabase's default privileges grant `anon` separately (00077). +revoke execute on function lorekit_match_retention( + timestamptz, timestamptz, integer, integer, integer, + timestamptz, timestamptz, integer, integer, integer, + timestamptz, integer, integer, integer +) from public, anon; +grant execute on function lorekit_match_retention( + timestamptz, timestamptz, integer, integer, integer, + timestamptz, timestamptz, integer, integer, integer, + timestamptz, integer, integer, integer +) to authenticated, service_role; + +-- ── 2. lorekit_memory_facets (00110 body) ───────────────────────────────── + +drop function if exists lorekit_memory_facets( + uuid, boolean, text, text[], text, text[], text, text[], text, text[], text, + text[], text, text[], text, text[], text, text[], text, text[], text, + text[], text, uuid[], timestamptz, timestamptz, integer, integer, integer, + integer, integer +); + +create or replace function lorekit_memory_facets( + p_user_id uuid, + p_archived boolean default false, + p_scope text default null, + p_tags text[] default null, + p_tags_mode text default 'any', + p_source_agent text[] default null, + p_source_agent_mode text default 'in', + p_trigger text[] default null, + p_trigger_mode text default 'in', + p_kind text[] default null, + p_kind_mode text default 'in', + p_host text[] default null, + p_host_mode text default 'in', + p_origin_repo text[] default null, + p_origin_repo_mode text default 'in', + p_origin_branch text[] default null, + p_origin_branch_mode text default 'in', + p_origin_pr text[] default null, + p_origin_pr_mode text default 'in', + -- Owner dimension (00064). `personal` plus one slug per member org with + -- visible rows. All optional: null/absent = not filtered. + p_owner text[] default null, + p_owner_mode text default 'in', + -- The CALLING KEY's restriction (00068), defaulted to unrestricted. + p_key_scopes text[] default '{}', + p_key_org_access text default 'all', + p_key_org_ids uuid[] default '{}', + -- 00108: the created_at window and the five retention thresholds, so a count + -- describes the same rows the list shows. Appended, null-defaulted. + p_created_since timestamptz default null, + p_created_until timestamptz default null, + p_min_age_days integer default null, + p_unseen_days integer default null, + p_max_seen_count integer default null, + p_max_read_count integer default null, + p_max_opened_count integer default null, + -- 00113: the Explorer's reverse directions — "opened within N days" and the + -- three "at least N" floors. Appended, null-defaulted. Read routes only: a + -- retention policy cannot carry them. + p_opened_within_days integer default null, + p_min_seen_count integer default null, + p_min_read_count integer default null, + p_min_opened_count integer default null +) +returns table (facet text, value text, count bigint) +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; + v_origin_pr integer[] := ( + select array_agg(x::integer) + from unnest(coalesce(p_origin_pr, '{}'::text[])) as x + where x ~ '^[0-9]+$' + ); + v_created_cutoff timestamptz := case + when p_min_age_days is null then null else now() - (p_min_age_days * interval '1 day') + end; + v_unseen_cutoff timestamptz := case + when p_unseen_days is null then null else now() - (p_unseen_days * interval '1 day') + end; + -- 00113: the "opened within N days" lower bound, resolved once per call for + -- the same reason as the two cutoffs above. + v_opened_since timestamptz := case + when p_opened_within_days is null then null else now() - (p_opened_within_days * interval '1 day') + end; + -- 00110: only `label`/tag has an `all` (AND / set-containment) mode — see + -- the module header. Every other dimension, and the tag facet in `in`/`nin` + -- mode, keep self-exclusion (unchanged). + v_tag_cooccur boolean := (p_tags_mode = 'all'); +begin + return query + with base as ( + select + m.tags, m.source_agent, m.trigger, m.kind, m.host, + m.origin_repo, m.origin_branch, m.origin_pr, + m.org_id, o.slug as org_slug, + -- Per-dimension match flag, from the shared predicates so it cannot + -- drift from lorekit_memory_activity's. A null filter is "not filtered" → + -- the helper returns true, so an untouched dimension never narrows. + lorekit_match_tags(m.tags, p_tags, p_tags_mode) as ok_tag, + lorekit_match_text(m.source_agent, p_source_agent, p_source_agent_mode) as ok_source_agent, + lorekit_match_text(m.trigger, p_trigger, p_trigger_mode) as ok_trigger, + lorekit_match_text(m.kind, p_kind, p_kind_mode) as ok_kind, + lorekit_match_text(m.host, p_host, p_host_mode) as ok_host, + lorekit_match_text(m.origin_repo, p_origin_repo, p_origin_repo_mode) as ok_origin_repo, + lorekit_match_text(m.origin_branch, p_origin_branch, p_origin_branch_mode) as ok_origin_branch, + lorekit_match_int (m.origin_pr, v_origin_pr, p_origin_pr_mode) as ok_origin_pr, + -- Owner: the computed identity is `personal` (org_id null) or the org slug. + -- Stays inline — it is not one of the three column helpers (00064). + (p_owner is null or case coalesce(p_owner_mode, 'in') + when 'nin' then ( + (case when m.org_id is null then 'personal' else o.slug end) is not null + and (case when m.org_id is null then 'personal' else o.slug end) <> all(p_owner) + ) + else ( + ('personal' = any(p_owner) and m.org_id is null) + or (m.org_id is not null and o.slug = any(p_owner)) + ) + end) as ok_owner + from memories m + -- A personal row has no org, so this is a LEFT join; org rows resolve to + -- their slug. Visible org rows are always the caller's own orgs (the + -- visibility predicate below admits them only via lorekit_member_org_ids), + -- so `o.slug` is never a slug the caller cannot see. + left join orgs o on o.id = m.org_id + where ( + (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)) + ) + -- The calling key's restriction, applied ONCE here in the row-visibility + -- predicate every emitted facet value is derived from — `origin_repo` is + -- a repository name by construction, so an unnarrowed facet list leaks + -- exactly what the scope catalog hides. + and lorekit_api_token_scope_allowed(p_key_scopes, m.scope) + and lorekit_api_token_org_allowed(p_key_org_access, p_key_org_ids, m.org_id) + and ( + case + when p_archived then m.archived_at is not null + else m.archived_at is null + and (m.expires_at is null or m.expires_at > now()) + end + ) + and (p_scope is null or m.scope = p_scope) + -- 00108. Here rather than as an `ok_*` flag: these narrow the population + -- every facet value is drawn from, they are not a dimension that can be + -- self-excluded. Half-open window, matching lorekit_memory_list. + and (p_created_since is null or m.created_at >= p_created_since) + and (p_created_until is null or m.created_at < p_created_until) + and lorekit_match_retention( + m.created_at, m.last_opened_at, m.seen_count, m.read_count, m.opened_count, + v_created_cutoff, v_unseen_cutoff, p_max_seen_count, p_max_read_count, p_max_opened_count, + v_opened_since, p_min_seen_count, p_min_read_count, p_min_opened_count + ) + ), cells as ( + -- 00110: every branch now scans the FULL `base` population (not just rows + -- that already satisfy the other flags) and carries a `matched` boolean + -- instead of a row-level WHERE — the `group by` below therefore enumerates + -- every value the dimension has, and the final `count` reflects only the + -- ones actually matching. This is what keeps a non-matching value at + -- `count = 0` instead of dropping it. + select 'tag'::text as facet, t.tag as value, + (b.ok_source_agent and b.ok_trigger and b.ok_kind and b.ok_host + and b.ok_origin_repo and b.ok_origin_branch and b.ok_origin_pr and b.ok_owner + -- Co-occurrence: fold the tag dimension's OWN flag back in only in + -- `all` mode, so another label's count becomes "carries the + -- selection AND this label" rather than the plain self-excluded + -- total. `not v_tag_cooccur` keeps `in`/`nin` mode's existing + -- self-exclusion untouched. + and (not v_tag_cooccur or b.ok_tag)) as matched + from base b + cross join lateral unnest(b.tags) as t(tag) + union all + select 'source_agent', b.source_agent, + (b.ok_tag and b.ok_trigger and b.ok_kind and b.ok_host + and b.ok_origin_repo and b.ok_origin_branch and b.ok_origin_pr and b.ok_owner) + from base b + where b.source_agent is not null + union all + select 'trigger', b.trigger, + (b.ok_tag and b.ok_source_agent and b.ok_kind and b.ok_host + and b.ok_origin_repo and b.ok_origin_branch and b.ok_origin_pr and b.ok_owner) + from base b + where b.trigger is not null + union all + select 'kind', b.kind, + (b.ok_tag and b.ok_source_agent and b.ok_trigger and b.ok_host + and b.ok_origin_repo and b.ok_origin_branch and b.ok_origin_pr and b.ok_owner) + from base b + where b.kind is not null + union all + select 'host', b.host, + (b.ok_tag and b.ok_source_agent and b.ok_trigger and b.ok_kind + and b.ok_origin_repo and b.ok_origin_branch and b.ok_origin_pr and b.ok_owner) + from base b + where b.host is not null + union all + select 'origin_repo', b.origin_repo, + (b.ok_tag and b.ok_source_agent and b.ok_trigger and b.ok_kind and b.ok_host + and b.ok_origin_branch and b.ok_origin_pr and b.ok_owner) + from base b + where b.origin_repo is not null + union all + select 'origin_branch', b.origin_branch, + (b.ok_tag and b.ok_source_agent and b.ok_trigger and b.ok_kind and b.ok_host + and b.ok_origin_repo and b.ok_origin_pr and b.ok_owner) + from base b + where b.origin_branch is not null + union all + select 'origin_pr', b.origin_pr::text, + (b.ok_tag and b.ok_source_agent and b.ok_trigger and b.ok_kind and b.ok_host + and b.ok_origin_repo and b.ok_origin_branch and b.ok_owner) + from base b + where b.origin_pr is not null + union all + -- Owner is the ONE dimension self-excluded here (every other flag, NOT + -- ok_owner), so a drilled-in owner still lists the alternative owner. + select 'owner', case when b.org_id is null then 'personal' else b.org_slug end, + (b.ok_tag and b.ok_source_agent and b.ok_trigger and b.ok_kind and b.ok_host + and b.ok_origin_repo and b.ok_origin_branch and b.ok_origin_pr) + from base b + ) + select c.facet, c.value, count(*) filter (where c.matched) as count + from cells c + where c.value is not null + and btrim(c.value) <> '' + group by c.facet, c.value + order by c.facet asc, count(*) filter (where c.matched) desc, c.value asc; +end; +$$; + +revoke execute on function lorekit_memory_facets( + uuid, boolean, text, text[], text, text[], text, text[], text, text[], text, + text[], text, text[], text, text[], text, text[], text, text[], text, + text[], text, uuid[], timestamptz, timestamptz, integer, integer, integer, + integer, integer, + integer, integer, integer, integer +) from public, anon; +grant execute on function lorekit_memory_facets( + uuid, boolean, text, text[], text, text[], text, text[], text, text[], text, + text[], text, text[], text, text[], text, text[], text, text[], text, + text[], text, uuid[], timestamptz, timestamptz, integer, integer, integer, + integer, integer, + integer, integer, integer, integer +) to authenticated, service_role; + +comment on function lorekit_memory_facets( + uuid, boolean, text, text[], text, text[], text, text[], text, text[], text, + text[], text, text[], text, text[], text, text[], text, text[], text, + text[], text, uuid[], timestamptz, timestamptz, integer, integer, integer, + integer, integer, + integer, integer, integer, integer +) is + 'Value catalog with counts for every filterable memory dimension (tag, + source_agent, trigger, kind, host, origin_repo, origin_branch, origin_pr, + owner) over the partition selected by p_archived, visible to the EFFECTIVE + caller and narrowed by the calling key''s own scope/org restriction. Each + dimension is counted with every OTHER dimension filter applied (cross-group + AND). WITHIN a dimension: self-exclusion (drill-down, 00057) for every + scalar dimension and for label/tag in `in`/`nin` (OR) mode; CO-OCCURRENCE + for label/tag in `all` (AND) mode (00110). Every dimension enumerates its + FULL value set so a value with zero matches still returns `count = 0` + (00110). The created_at window, the five retention thresholds (00108) and + the Explorer''s reverse directions (00113: opened_within_days and the three + min_* floors) are NOT self-excluded: they narrow the population every facet + value is drawn from, so a count matches the list under the same parameters. + q/key/expiring_within_days remain unmirrored, so with those active a count + is an upper bound.'; + +-- ── 3. lorekit_memory_activity (00108 body) ─────────────────────────────── + +drop function if exists lorekit_memory_activity( + uuid, text, timestamptz, timestamptz, text, text[], text, text[], text, + text[], text, text[], text, text[], text, text[], text, text[], text, text[], text, text[], text, + text[], text, uuid[], integer, integer, integer, integer, integer +); + +create or replace function lorekit_memory_activity( + p_user_id uuid, + p_bucket text default 'day', + p_since timestamptz default null, + p_until timestamptz default null, + p_scope text default null, + p_tags text[] default null, + p_tags_mode text default 'any', + p_source_agent text[] default null, + p_source_agent_mode text default 'in', + p_trigger text[] default null, + p_trigger_mode text default 'in', + p_kind text[] default null, + p_kind_mode text default 'in', + p_host text[] default null, + p_host_mode text default 'in', + p_origin_repo text[] default null, + p_origin_repo_mode text default 'in', + p_origin_branch text[] default null, + p_origin_branch_mode text default 'in', + p_origin_pr text[] default null, + p_origin_pr_mode text default 'in', + p_owner text[] default null, + p_owner_mode text default 'in', + -- The CALLING KEY's restriction (00068), defaulted to unrestricted. + p_key_scopes text[] default '{}', + p_key_org_access text default 'all', + p_key_org_ids uuid[] default '{}', + -- 00108: the five retention thresholds. No created_at window is added — this + -- function's own p_since/p_until already bound created_at, and a second + -- window would be two ways to say one thing. + p_min_age_days integer default null, + p_unseen_days integer default null, + p_max_seen_count integer default null, + p_max_read_count integer default null, + p_max_opened_count integer default null, + -- 00113: the Explorer's reverse directions — "opened within N days" and the + -- three "at least N" floors. Appended, null-defaulted. Read routes only: a + -- retention policy cannot carry them. + p_opened_within_days integer default null, + p_min_seen_count integer default null, + p_min_read_count integer default null, + p_min_opened_count integer default null +) +returns table (bucket timestamptz, scope text, count bigint) +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; + -- `origin_pr` is an INTEGER column: coerce the digits-only list numerically so + -- this route matches GET /memories on the same input (00057's rationale). A + -- non-numeric entry is dropped; a list reducing to empty applies no filter. + v_origin_pr integer[] := ( + select array_agg(x::integer) + from unnest(coalesce(p_origin_pr, '{}'::text[])) as x + where x ~ '^[0-9]+$' + ); + v_created_cutoff timestamptz := case + when p_min_age_days is null then null else now() - (p_min_age_days * interval '1 day') + end; + v_unseen_cutoff timestamptz := case + when p_unseen_days is null then null else now() - (p_unseen_days * interval '1 day') + end; + -- 00113: the "opened within N days" lower bound, resolved once per call for + -- the same reason as the two cutoffs above. + v_opened_since timestamptz := case + when p_opened_within_days is null then null else now() - (p_opened_within_days * interval '1 day') + end; +begin + if p_bucket is null or p_bucket not in ('hour', 'day') then + raise exception 'invalid bucket %, expected hour or day', p_bucket + using errcode = '22023'; + end if; + + return query + select date_trunc(p_bucket, m.created_at at time zone 'UTC') at time zone 'UTC' as bucket, + m.scope, + count(*) as count + from memories m + -- LEFT join for the owner predicate — a personal row has no org (00064). + left join orgs o on o.id = m.org_id + where ( + (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)) + ) + -- The calling key's own restriction, ANDed on top of the caller's + -- visibility and never instead of it (the lorekit_memory_scopes rule). + and lorekit_api_token_scope_allowed(p_key_scopes, m.scope) + and lorekit_api_token_org_allowed(p_key_org_access, p_key_org_ids, m.org_id) + and m.archived_at is null + and (m.expires_at is null or m.expires_at > now()) + and (p_since is null or m.created_at >= p_since) + and (p_until is null or m.created_at < p_until) + -- Scope is a hard filter, always applied (00057's treatment). + and (p_scope is null or m.scope = p_scope) + -- Dimension filters — AND across, OR within, from the shared predicates + -- (00066) so the semantics cannot drift from lorekit_memory_facets. No + -- self-exclusion here: a straight count applies every one directly. + and lorekit_match_tags(m.tags, p_tags, p_tags_mode) + and lorekit_match_text(m.source_agent, p_source_agent, p_source_agent_mode) + and lorekit_match_text(m.trigger, p_trigger, p_trigger_mode) + and lorekit_match_text(m.kind, p_kind, p_kind_mode) + and lorekit_match_text(m.host, p_host, p_host_mode) + and lorekit_match_text(m.origin_repo, p_origin_repo, p_origin_repo_mode) + and lorekit_match_text(m.origin_branch, p_origin_branch, p_origin_branch_mode) + and lorekit_match_int (m.origin_pr, v_origin_pr, p_origin_pr_mode) + -- 00108: the same thresholds the list applies, so the Explorer's stat + -- cards describe the rows underneath them. + and lorekit_match_retention( + m.created_at, m.last_opened_at, m.seen_count, m.read_count, m.opened_count, + v_created_cutoff, v_unseen_cutoff, p_max_seen_count, p_max_read_count, p_max_opened_count, + v_opened_since, p_min_seen_count, p_min_read_count, p_min_opened_count + ) + -- Owner: `personal` for org_id-null rows, else the org slug (00064). + and (p_owner is null or case coalesce(p_owner_mode, 'in') + when 'nin' then ( + (case when m.org_id is null then 'personal' else o.slug end) is not null + and (case when m.org_id is null then 'personal' else o.slug end) <> all(p_owner) + ) + else ( + ('personal' = any(p_owner) and m.org_id is null) + or (m.org_id is not null and o.slug = any(p_owner)) + ) + end) + group by 1, m.scope + order by 1 asc, m.scope asc; +end; +$$; + +revoke execute on function lorekit_memory_activity( + uuid, text, timestamptz, timestamptz, text, text[], text, text[], text, + text[], text, text[], text, text[], text, text[], text, text[], text, text[], text, text[], text, + text[], text, uuid[], integer, integer, integer, integer, integer, + integer, integer, integer, integer +) from public, anon; +grant execute on function lorekit_memory_activity( + uuid, text, timestamptz, timestamptz, text, text[], text, text[], text, + text[], text, text[], text, text[], text, text[], text, text[], text, text[], text, text[], text, + text[], text, uuid[], integer, integer, integer, integer, integer, + integer, integer, integer, integer +) to authenticated, service_role; + +comment on function lorekit_memory_activity( + uuid, text, timestamptz, timestamptz, text, text[], text, text[], text, + text[], text, text[], text, text[], text, text[], text, text[], text, text[], text, text[], text, + text[], text, uuid[], integer, integer, integer, integer, integer, + integer, integer, integer, integer +) is + 'Memories created per UTC hour/day per scope, visible to the EFFECTIVE caller + and narrowed by the calling key''s own scope/org restriction. Applies every + dimension filter directly (no self-exclusion — this is a straight count), + the five retention thresholds (00108) and the Explorer''s reverse directions + (00113), so the Explorer''s stat cards describe the list beneath them. + p_since/p_until already bound created_at, so no separate window parameter + exists.'; + +-- ── 4. lorekit_memory_pivot (00108 body) ────────────────────────────────── + +drop function if exists lorekit_memory_pivot( + text, text, uuid, boolean, text, integer, text[], text, text[], text, text[], text, + text[], text, text[], text, text[], text, text[], text, text[], text, text[], text, + text[], text, uuid[], timestamptz, timestamptz, integer, integer, integer, + integer, integer +); + +create or replace function lorekit_memory_pivot( + p_row_facet text, + p_col_facet text, + p_user_id uuid default null, + p_archived boolean default false, + p_scope text default null, + p_limit integer default 400, + p_tags text[] default null, + p_tags_mode text default 'any', + p_source_agent text[] default null, + p_source_agent_mode text default 'in', + p_trigger text[] default null, + p_trigger_mode text default 'in', + p_kind text[] default null, + p_kind_mode text default 'in', + p_host text[] default null, + p_host_mode text default 'in', + p_origin_repo text[] default null, + p_origin_repo_mode text default 'in', + p_origin_branch text[] default null, + p_origin_branch_mode text default 'in', + p_origin_pr text[] default null, + p_origin_pr_mode text default 'in', + p_owner text[] default null, + p_owner_mode text default 'in', + -- The CALLING KEY's restriction (00068), defaulted to unrestricted. + p_key_scopes text[] default '{}', + p_key_org_access text default 'all', + p_key_org_ids uuid[] default '{}', + -- 00108: the created_at window and the five retention thresholds. + p_created_since timestamptz default null, + p_created_until timestamptz default null, + p_min_age_days integer default null, + p_unseen_days integer default null, + p_max_seen_count integer default null, + p_max_read_count integer default null, + p_max_opened_count integer default null, + -- 00113: the Explorer's reverse directions — "opened within N days" and the + -- three "at least N" floors. Appended, null-defaulted. Read routes only: a + -- retention policy cannot carry them. + p_opened_within_days integer default null, + p_min_seen_count integer default null, + p_min_read_count integer default null, + p_min_opened_count integer default null +) +returns table (row_value text, col_value text, count bigint) +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; + v_origin_pr integer[] := ( + select array_agg(x::integer) + from unnest(coalesce(p_origin_pr, '{}'::text[])) as x + where x ~ '^[0-9]+$' + ); + v_created_cutoff timestamptz := case + when p_min_age_days is null then null else now() - (p_min_age_days * interval '1 day') + end; + v_unseen_cutoff timestamptz := case + when p_unseen_days is null then null else now() - (p_unseen_days * interval '1 day') + end; + -- 00113: the "opened within N days" lower bound, resolved once per call for + -- the same reason as the two cutoffs above. + v_opened_since timestamptz := case + when p_opened_within_days is null then null else now() - (p_opened_within_days * interval '1 day') + end; +begin + return query + with base as ( + select + m.tags, m.source_agent, m.trigger, m.kind, m.host, + m.origin_repo, m.origin_branch, m.origin_pr, + m.org_id, o.slug as org_slug, + lorekit_match_tags(m.tags, p_tags, p_tags_mode) as ok_tag, + lorekit_match_text(m.source_agent, p_source_agent, p_source_agent_mode) as ok_source_agent, + lorekit_match_text(m.trigger, p_trigger, p_trigger_mode) as ok_trigger, + lorekit_match_text(m.kind, p_kind, p_kind_mode) as ok_kind, + lorekit_match_text(m.host, p_host, p_host_mode) as ok_host, + lorekit_match_text(m.origin_repo, p_origin_repo, p_origin_repo_mode) as ok_origin_repo, + lorekit_match_text(m.origin_branch, p_origin_branch, p_origin_branch_mode) as ok_origin_branch, + lorekit_match_int (m.origin_pr, v_origin_pr, p_origin_pr_mode) as ok_origin_pr, + -- Owner stays inline for lorekit_memory_facets' reason: it is a + -- LEFT JOIN-computed identity, not one of the three column helpers. + (p_owner is null or case coalesce(p_owner_mode, 'in') + when 'nin' then ( + (case when m.org_id is null then 'personal' else o.slug end) is not null + and (case when m.org_id is null then 'personal' else o.slug end) <> all(p_owner) + ) + else ( + ('personal' = any(p_owner) and m.org_id is null) + or (m.org_id is not null and o.slug = any(p_owner)) + ) + end) as ok_owner + from memories m + left join orgs o on o.id = m.org_id + where ( + (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)) + ) + and lorekit_api_token_scope_allowed(p_key_scopes, m.scope) + and lorekit_api_token_org_allowed(p_key_org_access, p_key_org_ids, m.org_id) + and ( + case + when p_archived then m.archived_at is not null + else m.archived_at is null + and (m.expires_at is null or m.expires_at > now()) + end + ) + and (p_scope is null or m.scope = p_scope) + -- 00108. Row-visibility, not an `ok_*` flag — see lorekit_memory_facets. + and (p_created_since is null or m.created_at >= p_created_since) + and (p_created_until is null or m.created_at < p_created_until) + and lorekit_match_retention( + m.created_at, m.last_opened_at, m.seen_count, m.read_count, m.opened_count, + v_created_cutoff, v_unseen_cutoff, p_max_seen_count, p_max_read_count, p_max_opened_count, + v_opened_since, p_min_seen_count, p_min_read_count, p_min_opened_count + ) + ) + select rv.value, cv.value, count(*)::bigint + from base b + cross join lateral lorekit_memory_facet_value( + p_row_facet, b.tags, b.source_agent, b.trigger, b.kind, b.host, + b.origin_repo, b.origin_branch, b.origin_pr, b.org_id, b.org_slug + ) as rv(value) + cross join lateral lorekit_memory_facet_value( + p_col_facet, b.tags, b.source_agent, b.trigger, b.kind, b.host, + b.origin_repo, b.origin_branch, b.origin_pr, b.org_id, b.org_slug + ) as cv(value) + -- Each dimension's own filter applies UNLESS it is one of the two axes — + -- see the self-exclusion note at the top. One line per dimension so adding + -- a tenth is one line here, not a re-derivation. + where (p_row_facet = 'tag' or p_col_facet = 'tag' or b.ok_tag) + and (p_row_facet = 'source_agent' or p_col_facet = 'source_agent' or b.ok_source_agent) + and (p_row_facet = 'trigger' or p_col_facet = 'trigger' or b.ok_trigger) + and (p_row_facet = 'kind' or p_col_facet = 'kind' or b.ok_kind) + and (p_row_facet = 'host' or p_col_facet = 'host' or b.ok_host) + and (p_row_facet = 'origin_repo' or p_col_facet = 'origin_repo' or b.ok_origin_repo) + and (p_row_facet = 'origin_branch' or p_col_facet = 'origin_branch' or b.ok_origin_branch) + and (p_row_facet = 'origin_pr' or p_col_facet = 'origin_pr' or b.ok_origin_pr) + and (p_row_facet = 'owner' or p_col_facet = 'owner' or b.ok_owner) + -- A null column value yields no cell, matching lorekit_memory_facets. + and rv.value is not null + and cv.value is not null + group by rv.value, cv.value + -- Densest cells first, so a caller that has to truncate keeps the ones + -- worth drawing. Deterministic tie-break so a page boundary is stable. + order by count(*) desc, rv.value asc, cv.value asc + limit greatest(p_limit, 0); +end; +$$; + +revoke execute on function lorekit_memory_pivot( + text, text, uuid, boolean, text, integer, text[], text, text[], text, text[], text, + text[], text, text[], text, text[], text, text[], text, text[], text, text[], text, + text[], text, uuid[], timestamptz, timestamptz, integer, integer, integer, + integer, integer, + integer, integer, integer, integer +) from public, anon; +grant execute on function lorekit_memory_pivot( + text, text, uuid, boolean, text, integer, text[], text, text[], text, text[], text, + text[], text, text[], text, text[], text, text[], text, text[], text, text[], text, + text[], text, uuid[], timestamptz, timestamptz, integer, integer, integer, + integer, integer, + integer, integer, integer, integer +) to authenticated, service_role; + +comment on function lorekit_memory_pivot( + text, text, uuid, boolean, text, integer, text[], text, text[], text, text[], text, + text[], text, text[], text, text[], text, text[], text, text[], text, text[], text, + text[], text, uuid[], timestamptz, timestamptz, integer, integer, integer, + integer, integer, + integer, integer, integer, integer +) is + 'Cross-tabulation of two facet dimensions with a count per cell, visible to + the EFFECTIVE caller and narrowed by the calling key''s own scope/org + restriction. BOTH axes are self-excluded from the dimension filters. The + created_at window, the five retention thresholds (00108) and the Explorer''s + reverse directions (00113) are not self-excluded — they narrow the + population, matching lorekit_memory_facets.'; + +-- ── 5. lorekit_memory_list (00108 body) ─────────────────────────────────── +-- +-- The signature changes (four filters + the integer cursor), so unlike 00108 +-- this is a drop-and-create. 00105 recreated it with a bare `grant`, which +-- left the default-privilege EXECUTE for `anon` in place (00077's subject); +-- the revoke below restores what 00092/00094 established and what its three +-- siblings carry. It returns nothing to anon either way — the visibility +-- predicate admits no row for a null actor — but a security-definer reader +-- reachable from the anonymous surface is a difference nobody should have to +-- re-derive. + +drop function if exists lorekit_memory_list( + uuid, boolean, text, text, text, text, timestamptz, timestamptz, timestamptz, + timestamptz, text[], text, text[], text, text[], text, text[], text, text[], + text, text[], text, text[], text, text[], text, text[], text, text, + timestamptz, uuid, integer, text[], text, uuid[], integer, integer, integer, + integer, integer +); + +create or replace function lorekit_memory_list( + p_user_id uuid, + p_archived boolean default false, + p_scope text default null, + p_key text default null, + p_key_prefix text default null, + p_q text default null, + p_created_since timestamptz default null, + p_created_until timestamptz default null, + p_expires_after timestamptz default null, + p_expires_on_or_before timestamptz default null, + p_tags text[] default null, + p_tags_mode text default 'any', + p_source_agent text[] default null, + p_source_agent_mode text default 'in', + p_trigger text[] default null, + p_trigger_mode text default 'in', + p_kind text[] default null, + p_kind_mode text default 'in', + p_host text[] default null, + p_host_mode text default 'in', + p_origin_repo text[] default null, + p_origin_repo_mode text default 'in', + p_origin_branch text[] default null, + p_origin_branch_mode text default 'in', + p_origin_pr text[] default null, + p_origin_pr_mode text default 'in', + p_owner text[] default null, + p_owner_mode text default 'in', + p_sort text default 'updated_at', + p_cursor_ts timestamptz default null, + p_cursor_id uuid default null, + p_limit integer default 51, + p_key_scopes text[] default '{}', + p_key_org_access text default 'all', + p_key_org_ids uuid[] default '{}', + p_min_age_days integer default null, + p_unseen_days integer default null, + p_max_seen_count integer default null, + p_max_read_count integer default null, + p_max_opened_count integer default null, + -- 00113: the Explorer's reverse directions, appended and null-defaulted. + p_opened_within_days integer default null, + p_min_seen_count integer default null, + p_min_read_count integer default null, + p_min_opened_count integer default null, + -- 00113: the keyset cursor for `p_sort => 'opened_count'`. An integer sort + -- needs an integer cursor; `p_cursor_ts` keeps serving the two date sorts. + p_cursor_count integer default null +) +returns table ( + id uuid, + scope text, + key text, + value text, + tags text[], + source_agent text, + trigger text, + created_at timestamptz, + updated_at timestamptz, + expires_at timestamptz, + archived_at timestamptz, + origin_repo text, + origin_branch text, + origin_commit text, + origin_pr integer, + kind text, + host text, + seen_count integer, + read_count integer, + opened_count integer, + last_read_at timestamptz, + last_opened_at timestamptz, + org_id uuid, + created_by uuid, + updated_by uuid, + org_name text, + org_slug text, + total_count integer +) +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; + v_origin_pr integer[] := ( + select array_agg(x::integer) + from unnest(coalesce(p_origin_pr, '{}'::text[])) as x + where x ~ '^0*[0-9]{1,9}$' + ); + -- 00113: `opened_count` joins the two date sorts. Anything else still falls + -- back to `updated_at`, so an unknown value cannot reach the format() below. + v_sort text := case + when p_sort in ('created_at', 'opened_count') then p_sort else 'updated_at' + end; + -- Which bound parameter carries the keyset value: the integer cursor ($44) + -- for the count sort, the timestamp cursor ($29) for the date sorts. A fixed + -- placeholder chosen here, never caller text, so splicing it is safe. + v_cursor text := case when v_sort = 'opened_count' then '$44' else '$29' end; + -- Resolved once per call so lorekit_match_retention stays immutable (00108). + v_created_cutoff timestamptz := case + when p_min_age_days is null then null else now() - (p_min_age_days * interval '1 day') + end; + v_unseen_cutoff timestamptz := case + when p_unseen_days is null then null else now() - (p_unseen_days * interval '1 day') + end; + -- 00113: the "opened within N days" lower bound, resolved once per call for + -- the same reason as the two cutoffs above. + v_opened_since timestamptz := case + when p_opened_within_days is null then null else now() - (p_opened_within_days * interval '1 day') + end; +begin + return query execute format($q$ + select + m.id, m.scope, m.key, m.value, m.tags, m.source_agent, m.trigger, + m.created_at, m.updated_at, m.expires_at, m.archived_at, + m.origin_repo, m.origin_branch, m.origin_commit, m.origin_pr, + m.kind, m.host, m.seen_count, + m.read_count, m.opened_count, m.last_read_at, m.last_opened_at, + m.org_id, m.created_by, m.updated_by, + o.name as org_name, o.slug as org_slug, + (count(*) over ())::integer as total_count + from memories m + left join orgs o on o.id = m.org_id + where ( + ($1 is null and auth.role() = 'service_role') + or m.user_id = $1 + or m.org_id in (select lorekit_member_org_ids($1)) + ) + and lorekit_api_token_scope_allowed($32, m.scope) + and lorekit_api_token_org_allowed($33, $34, m.org_id) + and ( + case + when $2 then m.archived_at is not null + else m.archived_at is null + and (m.expires_at is null or m.expires_at > now()) + end + ) + and ($3 is null or m.scope = $3) + and ($4 is null or m.key = $4) + and ($5 is null or m.key ilike $5 || '%%') + and ($6 is null or m.key ilike '%%' || $6 || '%%' or m.value ilike '%%' || $6 || '%%') + and ($7 is null or m.created_at >= $7) + and ($8 is null or m.created_at < $8) + and ($9 is null or m.expires_at > $9) + and ($10 is null or m.expires_at <= $10) + and lorekit_match_tags(m.tags, $11, $12) + and lorekit_match_text(m.source_agent, $13, $14) + and lorekit_match_text(m.trigger, $15, $16) + and lorekit_match_text(m.kind, $17, $18) + and lorekit_match_text(m.host, $19, $20) + and lorekit_match_text(m.origin_repo, $21, $22) + and lorekit_match_text(m.origin_branch, $23, $24) + and lorekit_match_int (m.origin_pr, $25, $26) + and ($27 is null or case coalesce($28, 'in') + when 'nin' then ( + (case when m.org_id is null then 'personal' else o.slug end) is not null + and (case when m.org_id is null then 'personal' else o.slug end) <> all($27) + ) + else ( + ('personal' = any($27) and m.org_id is null) + or (m.org_id is not null and o.slug = any($27)) + ) + end) + and lorekit_match_retention( + m.created_at, m.last_opened_at, m.seen_count, m.read_count, m.opened_count, + $35, $36, $37, $38, $39, + $40, $41, $42, $43 + ) + and ( + %2$s is null + or m.%1$I < %2$s + or (m.%1$I = %2$s and m.id < $30) + ) + order by m.%1$I desc, m.id desc + limit $31 + $q$, v_sort, v_cursor) + using + v_actor, coalesce(p_archived, false), p_scope, p_key, p_key_prefix, p_q, + p_created_since, p_created_until, p_expires_after, p_expires_on_or_before, + p_tags, p_tags_mode, + p_source_agent, p_source_agent_mode, + p_trigger, p_trigger_mode, + p_kind, p_kind_mode, + p_host, p_host_mode, + p_origin_repo, p_origin_repo_mode, + p_origin_branch, p_origin_branch_mode, + v_origin_pr, p_origin_pr_mode, + p_owner, p_owner_mode, + p_cursor_ts, p_cursor_id, + greatest(coalesce(p_limit, 51), 1), + coalesce(p_key_scopes, '{}'::text[]), + coalesce(p_key_org_access, 'all'), + coalesce(p_key_org_ids, '{}'::uuid[]), + v_created_cutoff, v_unseen_cutoff, p_max_seen_count, p_max_read_count, + p_max_opened_count, + v_opened_since, p_min_seen_count, p_min_read_count, p_min_opened_count, + p_cursor_count; +end; +$$; + +revoke execute on function lorekit_memory_list( + uuid, boolean, text, text, text, text, timestamptz, timestamptz, timestamptz, + timestamptz, text[], text, text[], text, text[], text, text[], text, text[], + text, text[], text, text[], text, text[], text, text[], text, text, + timestamptz, uuid, integer, text[], text, uuid[], integer, integer, integer, + integer, integer, + integer, integer, integer, integer, integer +) from public, anon; +grant execute on function lorekit_memory_list( + uuid, boolean, text, text, text, text, timestamptz, timestamptz, timestamptz, + timestamptz, text[], text, text[], text, text[], text, text[], text, text[], + text, text[], text, text[], text, text[], text, text[], text, text, + timestamptz, uuid, integer, text[], text, uuid[], integer, integer, integer, + integer, integer, + integer, integer, integer, integer, integer +) to authenticated, service_role; + +comment on function lorekit_memory_list( + uuid, boolean, text, text, text, text, timestamptz, timestamptz, timestamptz, + timestamptz, text[], text, text[], text, text[], text, text[], text, text[], + text, text[], text, text[], text, text[], text, text[], text, text, + timestamptz, uuid, integer, text[], text, uuid[], integer, integer, integer, + integer, integer, + integer, integer, integer, integer, integer +) is + 'The keyset-paginated memory list behind GET /memories and POST /memories/list. + Sorts by updated_at (default), created_at, or opened_count (00113) — each + descending with id as the tie-break; p_cursor_ts carries the date sorts'' + keyset value and p_cursor_count the count sort''s. Applies the five retention + thresholds and the Explorer''s reverse directions through the shared + lorekit_match_retention, so its total_count agrees with _facets, _activity + and _pivot under the same parameters.'; + +-- ── 6. Retire the 10-argument helper ───────────────────────────────────── +-- +-- Last, once every caller above composes the 14-argument form. The callers +-- are plpgsql, whose bodies are resolved at call time and carry no catalog +-- dependency, so nothing blocks the drop; leaving it would be a second +-- predicate for someone to call by mistake. +drop function if exists lorekit_match_retention( + timestamptz, timestamptz, integer, integer, integer, + timestamptz, timestamptz, integer, integer, integer +); diff --git a/supabase/tests/migrations.test.sql b/supabase/tests/migrations.test.sql index fc5068e29..d90099997 100644 --- a/supabase/tests/migrations.test.sql +++ b/supabase/tests/migrations.test.sql @@ -42,6 +42,8 @@ values -- shared account every row an earlier section left behind would be counted -- too, and the numbers would drift as sections are added. ('00000000-0000-0000-0000-000000000000', '00000000-0000-0000-0000-0000000000a6', 'authenticated', 'authenticated', 'lk-mig-a6@test.local', now(), now()), + -- §113's own account, for §108's reason: exact counts out of four readers. + ('00000000-0000-0000-0000-000000000000', '00000000-0000-0000-0000-0000000000a7', 'authenticated', 'authenticated', 'lk-mig-a7@test.local', now(), now()), ('00000000-0000-0000-0000-000000000000', '00000000-0000-0000-0000-0000000000b2', 'authenticated', 'authenticated', 'lk-mig-b@test.local', now(), now()), ('00000000-0000-0000-0000-000000000000', '00000000-0000-0000-0000-0000000000c3', 'authenticated', 'authenticated', 'lk-mig-c@test.local', now(), now()), ('00000000-0000-0000-0000-000000000000', '00000000-0000-0000-0000-0000000000d4', 'authenticated', 'authenticated', 'lk-mig-d@test.local', now(), now()), @@ -9939,7 +9941,8 @@ $$; -- header records this happening). Asserted for all four readers at once. do $$ declare - r record; + r record; + v_helper_sig text; begin for r in select p.proname, count(*) as n @@ -9959,13 +9962,17 @@ begin -- The shared helper carries its family's grants. A `create or replace` of a -- NEW function starts from EXECUTE-to-public, so this is the line that keeps -- it from being the one `lorekit_match_*` reachable by anon. - assert not has_function_privilege('anon', - 'lorekit_match_retention(timestamptz, timestamptz, integer, integer, integer, ' - 'timestamptz, timestamptz, integer, integer, integer)', 'EXECUTE'), + -- + -- The signature is DERIVED from the catalog (§70's reason): 00113 widened the + -- helper from 10 to 14 arguments, and a hardcoded list would either break on + -- the next widening or keep passing against a stale overload. The loop above + -- has already proved there is exactly one. + select p.oid::regprocedure::text into v_helper_sig + from pg_proc p join pg_namespace n on n.oid = p.pronamespace + where n.nspname = 'public' and p.proname = 'lorekit_match_retention'; + assert not has_function_privilege('anon', v_helper_sig, 'EXECUTE'), '108 AC-7b: anon must NOT have EXECUTE on lorekit_match_retention'; - assert has_function_privilege('authenticated', - 'lorekit_match_retention(timestamptz, timestamptz, integer, integer, integer, ' - 'timestamptz, timestamptz, integer, integer, integer)', 'EXECUTE'), + assert has_function_privilege('authenticated', v_helper_sig, 'EXECUTE'), '108 AC-7c: authenticated MUST have EXECUTE on lorekit_match_retention'; -- It must stay IMMUTABLE: that is what lets the planner fold it into an @@ -10645,6 +10652,252 @@ begin end; $$; +-- ── 113. the Explorer's reverse directions and the "most chosen" sort (00113) ── +-- 00108 made the five retention thresholds reach all four readers. Every one of +-- them points the retention way — "N or fewer", "not opened for N days" — so +-- the Explorer could show the dead weight and never the load-bearing lore. +-- 00113 adds the mirror of each (min_seen_count / min_read_count / +-- min_opened_count / opened_within_days) through the SAME widened helper, and +-- an `opened_count` sort on the list. +-- +-- Four lessons on a fresh account, so every count is exact: +-- 112-hot seen 5 read 300 opened 12 last opened 1 day ago host h1 +-- 112-warm seen 2 read 120 opened 3 last opened 40 days ago host h1 +-- 112-tie seen 1 read 50 opened 3 last opened 2 days ago host h2 +-- 112-cold seen 1 read 10 opened 0 never opened, written 1d host h2 +-- +-- AC-1: each floor narrows the list, a facet count, activity and the pivot by +-- the SAME rows — and a facet value whose rows all fall below it goes. +-- AC-2: a floor and its ceiling compose into a range. +-- AC-3: opened_within_days keeps the recently-opened rows and drops BOTH the +-- long-unopened one AND the never-opened one — unlike unseen_days, it +-- does not age a never-opened lesson from created_at. +-- AC-4: sort => 'opened_count' orders by opened_count desc with an id +-- tie-break, and a keyset page built from p_cursor_count/p_cursor_id +-- continues it without overlap. p_cursor_ts does not leak into it, and an +-- unknown sort still falls back to updated_at. +-- AC-5: the new params all-null are identical to omitting them. +-- AC-6: one overload per reader, the 10-argument helper is gone, and every +-- reader is off the anonymous surface (00113 restores the list's revoke). +do $$ +declare + v_user uuid := '00000000-0000-0000-0000-0000000000a7'; + v_n bigint; + v_h1 bigint; + v_h2 bigint; + v_expected text[]; + v_got text[]; + v_page1 text[]; + v_page2 text[]; + v_last_id uuid; + v_last_cnt integer; +begin + set local role service_role; + perform set_config('request.jwt.claims', + format('{"sub":"%s","role":"service_role"}', v_user), true); + + insert into memories (user_id, scope, key, value, kind, host, created_at, + seen_count, read_count, opened_count, last_opened_at) values + (v_user, 'global', '113-hot', 'v', 'lesson', 'h1', now() - interval '100 days', 5, 300, 12, now() - interval '1 day'), + (v_user, 'global', '113-warm', 'v', 'lesson', 'h1', now() - interval '100 days', 2, 120, 3, now() - interval '40 days'), + (v_user, 'global', '113-tie', 'v', 'lesson', 'h2', now() - interval '100 days', 1, 50, 3, now() - interval '2 days'), + (v_user, 'global', '113-cold', 'v', 'lesson', 'h2', now() - interval '1 day', 1, 10, 0, null); + + -- ── AC-1: each floor, on all four readers ─────────────────────────────── + select count(*) into v_n from lorekit_memory_list( + p_user_id => v_user, p_scope => 'global', p_min_opened_count => 3, p_limit => 100); + assert v_n = 3, format('113 AC-1a: min_opened_count => 3 must list hot, warm and tie, got %s', v_n); + + select count into v_h1 from lorekit_memory_facets( + p_user_id => v_user, p_scope => 'global', p_min_opened_count => 3) + where facet = 'host' and value = 'h1'; + select count into v_h2 from lorekit_memory_facets( + p_user_id => v_user, p_scope => 'global', p_min_opened_count => 3) + where facet = 'host' and value = 'h2'; + assert v_h1 = 2 and v_h2 = 1, + format('113 AC-1b: the host facet must count the rows the list returns (h1=2, h2=1), ' + 'got h1=%s h2=%s', v_h1, v_h2); + + select coalesce(sum(count), 0) into v_n + from lorekit_memory_activity(p_user_id => v_user, p_scope => 'global', p_min_opened_count => 3); + assert v_n = 3, format('113 AC-1c: activity must count the same 3 rows, got %s', v_n); + + select coalesce(sum(count), 0) into v_n + from lorekit_memory_pivot('host', 'kind', p_user_id => v_user, p_scope => 'global', + p_min_opened_count => 3); + assert v_n = 3, format('113 AC-1d: the pivot must count the same 3 rows, got %s', v_n); + + -- Delivered: only hot and warm reach 100, so every h2 row falls below the + -- floor and the VALUE disappears (retention-style conditions narrow `base`). + select count(*) into v_n from lorekit_memory_list( + p_user_id => v_user, p_scope => 'global', p_min_read_count => 100, p_limit => 100); + assert v_n = 2, format('113 AC-1e: min_read_count => 100 must list hot and warm, got %s', v_n); + + select count(*) into v_n from lorekit_memory_facets( + p_user_id => v_user, p_scope => 'global', p_min_read_count => 100) + where facet = 'host' and value = 'h2'; + assert v_n = 0, + '113 AC-1f: a host whose every row is below the floor must DISAPPEAR from the catalog'; + + -- Recurrence. + select count(*) into v_n from lorekit_memory_list( + p_user_id => v_user, p_scope => 'global', p_min_seen_count => 2, p_limit => 100); + assert v_n = 2, format('113 AC-1g: min_seen_count => 2 must list hot and warm, got %s', v_n); + + select coalesce(sum(count), 0) into v_n + from lorekit_memory_activity(p_user_id => v_user, p_scope => 'global', p_min_seen_count => 2); + assert v_n = 2, format('113 AC-1h: activity must agree on min_seen_count, got %s', v_n); + + -- ── AC-2: a floor and a ceiling compose ───────────────────────────────── + select count(*) into v_n from lorekit_memory_list( + p_user_id => v_user, p_scope => 'global', + p_min_opened_count => 1, p_max_opened_count => 5, p_limit => 100); + assert v_n = 2, format('113 AC-2: chosen 1..5 must list warm and tie, got %s', v_n); + + -- ── AC-3: opened_within_days ──────────────────────────────────────────── + select array_agg(key order by key) into v_got from lorekit_memory_list( + p_user_id => v_user, p_scope => 'global', p_opened_within_days => 7, p_limit => 100); + assert v_got = array['113-hot', '113-tie'], + format('113 AC-3a: opened_within_days => 7 must keep exactly hot and tie (cold was ' + 'never opened, warm 40 days ago), got %s', v_got); + + select count into v_h1 from lorekit_memory_facets( + p_user_id => v_user, p_scope => 'global', p_opened_within_days => 7) + where facet = 'host' and value = 'h1'; + select count into v_h2 from lorekit_memory_facets( + p_user_id => v_user, p_scope => 'global', p_opened_within_days => 7) + where facet = 'host' and value = 'h2'; + assert v_h1 = 1 and v_h2 = 1, + format('113 AC-3b: facets must apply opened_within_days, got h1=%s h2=%s', v_h1, v_h2); + + select coalesce(sum(count), 0) into v_n + from lorekit_memory_activity(p_user_id => v_user, p_scope => 'global', p_opened_within_days => 7); + assert v_n = 2, format('113 AC-3c: activity must apply opened_within_days, got %s', v_n); + + select coalesce(sum(count), 0) into v_n + from lorekit_memory_pivot('host', 'kind', p_user_id => v_user, p_scope => 'global', + p_opened_within_days => 7); + assert v_n = 2, format('113 AC-3d: the pivot must apply opened_within_days, got %s', v_n); + + -- The contrast with unseen_days, whose never-opened fallback ages cold from + -- its creation one day ago: cold matches NEITHER direction at 7 days, which + -- is the honest answer for a lesson nothing has ever opened. + select array_agg(key order by key) into v_got from lorekit_memory_list( + p_user_id => v_user, p_scope => 'global', p_unseen_days => 7, p_limit => 100); + assert v_got = array['113-warm'], + format('113 AC-3e: unseen_days => 7 must match only warm, got %s', v_got); + + -- ── AC-4: the opened_count sort ───────────────────────────────────────── + select array_agg(key order by opened_count desc, id desc) into v_expected + from memories where user_id = v_user and scope = 'global'; + + select array_agg(l.key order by l.ord) into v_got + from lorekit_memory_list(p_user_id => v_user, p_scope => 'global', + p_sort => 'opened_count', p_limit => 100) + with ordinality as l(id, scope, key, value, tags, source_agent, trigger, + created_at, updated_at, expires_at, archived_at, + origin_repo, origin_branch, origin_commit, origin_pr, + kind, host, seen_count, read_count, opened_count, + last_read_at, last_opened_at, org_id, created_by, + updated_by, org_name, org_slug, total_count, ord); + assert v_got = v_expected, + format('113 AC-4a: sort => opened_count must order by opened_count desc, id desc. ' + 'expected %s, got %s', v_expected, v_got); + assert v_got[1] = '113-hot' and v_got[4] = '113-cold', + format('113 AC-4b: the most-chosen lesson leads and the never-chosen one trails, got %s', v_got); + + -- Page 1 of 2, then the cursor built from page 1's last row. + select array_agg(l.key order by l.ord) into v_page1 + from lorekit_memory_list(p_user_id => v_user, p_scope => 'global', + p_sort => 'opened_count', p_limit => 2) + with ordinality as l(id, scope, key, value, tags, source_agent, trigger, + created_at, updated_at, expires_at, archived_at, + origin_repo, origin_branch, origin_commit, origin_pr, + kind, host, seen_count, read_count, opened_count, + last_read_at, last_opened_at, org_id, created_by, + updated_by, org_name, org_slug, total_count, ord); + select id, opened_count into v_last_id, v_last_cnt + from memories where user_id = v_user and key = v_page1[2]; + + select array_agg(l.key order by l.ord) into v_page2 + from lorekit_memory_list(p_user_id => v_user, p_scope => 'global', + p_sort => 'opened_count', p_limit => 100, + p_cursor_count => v_last_cnt, p_cursor_id => v_last_id) + with ordinality as l(id, scope, key, value, tags, source_agent, trigger, + created_at, updated_at, expires_at, archived_at, + origin_repo, origin_branch, origin_commit, origin_pr, + kind, host, seen_count, read_count, opened_count, + last_read_at, last_opened_at, org_id, created_by, + updated_by, org_name, org_slug, total_count, ord); + assert v_page1 || v_page2 = v_expected, + format('113 AC-4c: page 2 must continue page 1 exactly — no gap, no overlap — ' + 'including inside the run of equal counts. page1=%s page2=%s expected=%s', + v_page1, v_page2, v_expected); + + -- A timestamp cursor means nothing to the count sort; it must not narrow it. + select count(*) into v_n from lorekit_memory_list( + p_user_id => v_user, p_scope => 'global', p_sort => 'opened_count', + p_cursor_ts => now() - interval '1000 days', p_cursor_id => v_last_id, p_limit => 100); + assert v_n = 4, + format('113 AC-4d: p_cursor_ts must be ignored under the opened_count sort, got %s rows', v_n); + + -- An unknown sort still falls back to updated_at rather than erroring. + select count(*) into v_n from lorekit_memory_list( + p_user_id => v_user, p_scope => 'global', p_sort => 'bogus', p_limit => 100); + assert v_n = 4, format('113 AC-4e: an unknown sort must fall back, not fail, got %s', v_n); + + -- ── AC-5: all-null is a no-op ─────────────────────────────────────────── + select count(*) into v_n from lorekit_memory_list( + p_user_id => v_user, p_scope => 'global', + p_opened_within_days => null, p_min_seen_count => null, p_min_read_count => null, + p_min_opened_count => null, p_cursor_count => null, p_limit => 100); + assert v_n = 4, format('113 AC-5a: all-null new list params must change nothing, got %s', v_n); + + select count into v_h1 from lorekit_memory_facets( + p_user_id => v_user, p_scope => 'global', + p_opened_within_days => null, p_min_seen_count => null, p_min_read_count => null, + p_min_opened_count => null) + where facet = 'host' and value = 'h1'; + assert v_h1 = 2, format('113 AC-5b: all-null new facet params must change nothing, got %s', v_h1); + + reset role; + perform set_config('request.jwt.claims', '', true); +end; +$$; + +-- AC-6: the catalog side. +do $$ +declare + r record; +begin + assert to_regprocedure( + 'lorekit_match_retention(timestamptz, timestamptz, integer, integer, integer, ' + 'timestamptz, timestamptz, integer, integer, integer)') is null, + '113 AC-6a: the 10-argument lorekit_match_retention must be dropped — a second ' + 'predicate is somewhere for a caller to drift to'; + + for r in + select p.proname, p.oid::regprocedure::text as sig, p.pronargs + from pg_proc p join pg_namespace n on n.oid = p.pronamespace + where n.nspname = 'public' + and p.proname in ('lorekit_memory_list', 'lorekit_memory_facets', + 'lorekit_memory_activity', 'lorekit_memory_pivot', + 'lorekit_match_retention') + loop + assert not has_function_privilege('anon', r.sig, 'EXECUTE'), + format('113 AC-6b: anon must NOT have EXECUTE on %s', r.sig); + assert has_function_privilege('authenticated', r.sig, 'EXECUTE'), + format('113 AC-6c: authenticated MUST have EXECUTE on %s', r.sig); + assert has_function_privilege('service_role', r.sig, 'EXECUTE'), + format('113 AC-6d: service_role MUST have EXECUTE on %s', r.sig); + end loop; + + assert (select pronargs from pg_proc p join pg_namespace n on n.oid = p.pronamespace + where n.nspname = 'public' and p.proname = 'lorekit_match_retention') = 14, + '113 AC-6e: lorekit_match_retention must take the 14 arguments 00113 gave it'; +end; +$$; + rollback; \echo 'migrations.test.sql: all assertions passed'