From a1ac06a6d0537c34216032dd638fe683c70b2b04 Mon Sep 17 00:00:00 2001 From: "mads.thines" Date: Tue, 20 Oct 2026 17:40:21 +0000 Subject: [PATCH 1/9] feat(db): add p_exclude_tags to the four memory readers An always-applied 'carries none of these labels' conjunct on lorekit_memory_list/_facets/_activity/_pivot, so 'tagged a AND NOT tagged b' is expressible alongside any tags_mode. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> --- .../00114_exclude_tags_dimension.sql | 975 ++++++++++++++++++ 1 file changed, 975 insertions(+) create mode 100644 supabase/migrations/00114_exclude_tags_dimension.sql diff --git a/supabase/migrations/00114_exclude_tags_dimension.sql b/supabase/migrations/00114_exclude_tags_dimension.sql new file mode 100644 index 000000000..88cc5f078 --- /dev/null +++ b/supabase/migrations/00114_exclude_tags_dimension.sql @@ -0,0 +1,975 @@ +-- ═════════════════════════════════════════════════════════════════════════ +-- 00114 — `exclude_tags`: "tagged a AND NOT tagged b". +-- +-- THE GAP. A label filter is ONE condition: a list plus a mode — `all` +-- (containment), `any` (overlap) or `none` (no overlap). That can say "carries +-- perf", or "carries none of flaky", but never both at once, because the +-- second needs a different mode over a different list. The Explorer's detail +-- panel now offers "filter out" beside "filter in" on every tag chip, and a +-- reader who includes one label and excludes another expects both to hold. +-- +-- THE CHANGE. One appended, null-defaulted parameter on each of the four +-- readers: +-- +-- p_exclude_tags text[] the row carries NONE of these labels +-- +-- It is an independent conjunct — always ANDed, whatever `p_tags_mode` is — +-- and it reuses `lorekit_match_tags(…, 'none')`, so "carries none of" has +-- exactly one definition in the schema: NOT(overlap), never NOT(containment). +-- +-- ── DESIGN ─────────────────────────────────────────────────────────────── +-- +-- NOT A NEW MODE. Folding the exclusion into `p_tags_mode` (an `all_except`, +-- say) would need a second value list on the same parameter, and would leave +-- `any` + exclusion inexpressible. A separate list composes with every mode. +-- +-- THE TAG FACET NEVER SELF-EXCLUDES IT. Self-exclusion (00057) exists so a +-- drilled-in dimension keeps showing its alternatives: the count beside an +-- unselected label is what selecting it would yield. Selecting a label adds it +-- to the INCLUDE condition, and the result still carries none of the excluded +-- labels — so the exclusion belongs in every count, the tag facet's included. +-- It is a match flag there rather than a `base` predicate, so an excluded label +-- stays in the catalog at `count = 0` (00110's rule) and the menu can still +-- offer to un-exclude it. The pivot narrows `base` instead: a matrix emits no +-- zero cells, so the two are equivalent there. +-- +-- A LONE EXCLUSION IS STILL `tags_mode => 'none'`. The dashboard only sends +-- `exclude_tags` when an include condition is ALSO active; on its own an +-- exclusion keeps travelling as it always has, so retention policies (which +-- never gain this parameter) and every existing link mean what they meant. +-- +-- ── SIGNATURES ─────────────────────────────────────────────────────────── +-- +-- `create or replace` cannot change a signature, so each reader is dropped at +-- its EXACT 00113 argument list first and recreated with the parameter +-- appended — §108 and §113 assert exactly one overload of each survives. +-- +-- §114 in `migrations.test.sql` is the executable proof. +-- ═════════════════════════════════════════════════════════════════════════ + +-- ── 1. lorekit_memory_facets (00113 body + p_exclude_tags) ───────────────── + +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, + 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, + -- 00114: labels the row must carry NONE of — an always-applied conjunct, + -- independent of p_tags/p_tags_mode, so "tagged a AND NOT tagged b" is + -- expressible. Appended, null-defaulted: null = not filtered. + p_exclude_tags text[] 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. + -- 00114: `ok_tag` is the WHOLE tag predicate — the include condition AND + -- the exclusion — because that is what every OTHER dimension's count must + -- apply. The tag facet itself self-excludes the include condition (or + -- folds it back in for co-occurrence) but never the exclusion, so it reads + -- `ok_tag_exclusion` on its own. + (lorekit_match_tags(m.tags, p_tags, p_tags_mode) + and lorekit_match_tags(m.tags, p_exclude_tags, 'none')) as ok_tag, + lorekit_match_tags(m.tags, p_exclude_tags, 'none') as ok_tag_exclusion, + 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) + -- 00114: an excluded label is never self-excluded. Selecting another + -- label still yields only rows carrying none of the excluded ones, + -- and an excluded label stays listed at `count = 0` (00110's rule) + -- rather than vanishing from the menu. + and b.ok_tag_exclusion) 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, + text[]) 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, + text[]) 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, + text[]) 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. p_exclude_tags (00114) is an always-applied conjunct on + every count, the tag facet''s included: never self-excluded, so an excluded + label stays listed at count = 0.'; + +-- ── 2. lorekit_memory_activity (00113 body + p_exclude_tags) ─────────────── + +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, + 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, + -- 00114: labels the row must carry NONE of — an always-applied conjunct, + -- independent of p_tags/p_tags_mode, so "tagged a AND NOT tagged b" is + -- expressible. Appended, null-defaulted: null = not filtered. + p_exclude_tags text[] 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_tags(m.tags, p_exclude_tags, 'none') + 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, + text[]) 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, + text[]) 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, + text[]) 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. p_exclude_tags (00114) is applied like every other filter.'; + +-- ── 3. lorekit_memory_pivot (00113 body + p_exclude_tags) ────────────────── + +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, + 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, + -- 00114: labels the row must carry NONE of — an always-applied conjunct, + -- independent of p_tags/p_tags_mode, so "tagged a AND NOT tagged b" is + -- expressible. Appended, null-defaulted: null = not filtered. + p_exclude_tags text[] 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 + ) + -- 00114: in the population, not an axis-excludable `ok_*` flag — with a + -- tag axis an excluded label must still yield no cell, and a matrix + -- emits no zero cells, so narrowing `base` is the exact equivalent. + and lorekit_match_tags(m.tags, p_exclude_tags, 'none') + ) + 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, + text[]) 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, + text[]) 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, + text[]) 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) and p_exclude_tags (00114) are not self-excluded + — they narrow the population, matching lorekit_memory_facets.'; + +-- ── 4. lorekit_memory_list (00113 body + p_exclude_tags) ─────────────────── + +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, + 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, + -- 00114: labels the row must carry NONE of — an always-applied conjunct, + -- independent of p_tags/p_tags_mode, so "tagged a AND NOT tagged b" is + -- expressible. Appended, null-defaulted: null = not filtered. + p_exclude_tags text[] 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_tags(m.tags, $45, 'none') + 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, + p_exclude_tags; +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, + text[]) 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, + text[]) 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, + text[]) 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. p_exclude_tags (00114): the row carries + none of these labels, ANDed with whatever p_tags/p_tags_mode ask.'; From 06b6dd28e552400e19223178f96ce6af133654c7 Mon Sep 17 00:00:00 2001 From: "mads.thines" Date: Tue, 20 Oct 2026 17:41:47 +0000 Subject: [PATCH 2/9] feat(api): accept exclude_tags on memory list/facets/activity/pivot Both transports (GET query + POST body) decode it into MemoryDimensions and the four handlers forward it as p_exclude_tags. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> --- packages/schemas/src/domain/memory.ts | 18 ++++++++++++++++++ packages/schemas/src/shared/dimensions.spec.ts | 11 +++++++++++ packages/schemas/src/shared/dimensions.ts | 10 ++++++++++ .../functions/_shared/schemas/dimensions.ts | 10 ++++++++++ supabase/functions/_shared/schemas/memory.ts | 18 ++++++++++++++++++ .../functions/memories/handlers/activity.ts | 1 + supabase/functions/memories/handlers/facets.ts | 1 + supabase/functions/memories/handlers/list.ts | 1 + supabase/functions/memories/handlers/pivot.ts | 1 + 9 files changed, 71 insertions(+) diff --git a/packages/schemas/src/domain/memory.ts b/packages/schemas/src/domain/memory.ts index c8429b7e0..546340064 100644 --- a/packages/schemas/src/domain/memory.ts +++ b/packages/schemas/src/domain/memory.ts @@ -259,6 +259,16 @@ export const ListMemoriesQuerySchema = z.object({ key_prefix: z.string().min(1).max(512).optional(), tags: z.string().optional(), tags_mode: TagsModeSchema.optional().default('any'), + /** + * Labels the row must carry NONE of (comma-separated, like `tags`). + * + * An independent conjunct, ANDed with whatever `tags`/`tags_mode` ask — which + * is what makes "tagged `perf` AND NOT tagged `flaky`" expressible: one + * `tags` list has one mode, so it can include or exclude but never both. + * Same `not.ov` semantics as `tags_mode=none` (carries none of them), never + * the negation of containment. Migration 00114. + */ + exclude_tags: z.string().optional(), /** * Case-insensitive substring match against `key` OR `value`. * @@ -547,6 +557,8 @@ export const ListFacetsQuerySchema = z.object({ scope: RawScopeSchema.optional(), tags: z.string().optional(), tags_mode: TagsModeSchema.optional().default('any'), + /** Labels the row must carry NONE of — see `ListMemoriesQuerySchema.exclude_tags`. */ + exclude_tags: z.string().optional(), source_agent: ValueListSchema.optional(), source_agent_mode: ScalarFilterModeSchema.optional().default('in'), trigger: ValueListSchema.optional(), @@ -638,6 +650,8 @@ export const PivotQuerySchema = z.object({ scope: RawScopeSchema.optional(), tags: z.string().optional(), tags_mode: TagsModeSchema.optional().default('any'), + /** Labels the row must carry NONE of — see `ListMemoriesQuerySchema.exclude_tags`. */ + exclude_tags: z.string().optional(), source_agent: ValueListSchema.optional(), source_agent_mode: ScalarFilterModeSchema.optional().default('in'), trigger: ValueListSchema.optional(), @@ -722,6 +736,8 @@ export const ActivityQuerySchema = z.object({ scope: RawScopeSchema.optional(), tags: z.string().optional(), tags_mode: TagsModeSchema.optional().default('any'), + /** Labels the row must carry NONE of — see `ListMemoriesQuerySchema.exclude_tags`. */ + exclude_tags: z.string().optional(), source_agent: ValueListSchema.optional(), source_agent_mode: ScalarFilterModeSchema.optional().default('in'), trigger: ValueListSchema.optional(), @@ -1065,6 +1081,8 @@ const DimensionValuesSchema = z const dimensionBodyFields = { tags: DimensionValuesSchema.optional(), tags_mode: TagsModeSchema.optional().default('any'), + /** Labels the row must carry NONE of — see `ListMemoriesQuerySchema.exclude_tags`. */ + exclude_tags: DimensionValuesSchema.optional(), source_agent: DimensionValuesSchema.optional(), source_agent_mode: ScalarFilterModeSchema.optional().default('in'), trigger: DimensionValuesSchema.optional(), diff --git a/packages/schemas/src/shared/dimensions.spec.ts b/packages/schemas/src/shared/dimensions.spec.ts index d7ba53c21..1aafaf6b6 100644 --- a/packages/schemas/src/shared/dimensions.spec.ts +++ b/packages/schemas/src/shared/dimensions.spec.ts @@ -37,6 +37,17 @@ describe('dimensionsFromQuery / dimensionsFromBody', () => { const d = dimensionsFromQuery({}); expect(d.host.values).toEqual([]); expect(d.tags.values).toEqual([]); + expect(d.exclude_tags).toEqual([]); + }); + + it('decode exclude_tags beside tags, independent of tags_mode, on both sides', () => { + // The exclusion is its own conjunct (00114): an include condition in ANY + // mode composes with it, so neither decoder may fold one into the other. + const fromQuery = dimensionsFromQuery({ tags: 'perf', tags_mode: 'all', exclude_tags: ' flaky ,ci,flaky' }); + const fromBody = dimensionsFromBody({ tags: ['perf'], tags_mode: 'all', exclude_tags: [' flaky ', 'ci', 'flaky'] }); + expect(fromBody).toEqual(fromQuery); + expect(fromQuery.tags).toEqual({ values: ['perf'], mode: 'all' }); + expect(fromQuery.exclude_tags).toEqual(['flaky', 'ci']); }); it('trim, drop empties and dedupe on both sides', () => { diff --git a/packages/schemas/src/shared/dimensions.ts b/packages/schemas/src/shared/dimensions.ts index 663b1a058..a339f19d2 100644 --- a/packages/schemas/src/shared/dimensions.ts +++ b/packages/schemas/src/shared/dimensions.ts @@ -48,6 +48,13 @@ export interface Dimension { */ export interface MemoryDimensions { tags: Dimension; + /** + * Labels the row must carry NONE of, ANDed with `tags` whatever its mode + * (migration 00114). A bare list, not a `Dimension`: its combination is fixed + * ("carries none of"), so a `mode` would be a field with one legal value. + * Empty means "not filtered", like every dimension's `values`. + */ + exclude_tags: string[]; source_agent: Dimension; trigger: Dimension; kind: Dimension; @@ -96,6 +103,7 @@ function digitsOnly(values: readonly string[]): string[] { export interface DimensionQueryParams { tags?: string | undefined; tags_mode?: TagsMode | undefined; + exclude_tags?: string | undefined; source_agent?: string | undefined; source_agent_mode?: ScalarFilterMode | undefined; trigger?: string | undefined; @@ -118,6 +126,7 @@ export interface DimensionQueryParams { export interface DimensionBodyFields { tags?: readonly string[] | undefined; tags_mode?: TagsMode | undefined; + exclude_tags?: readonly string[] | undefined; source_agent?: readonly string[] | undefined; source_agent_mode?: ScalarFilterMode | undefined; trigger?: readonly string[] | undefined; @@ -146,6 +155,7 @@ function build( }); return { tags: { values: values('tags'), mode: params.tags_mode ?? 'any' }, + exclude_tags: values('exclude_tags'), source_agent: scalar('source_agent'), trigger: scalar('trigger'), kind: scalar('kind'), diff --git a/supabase/functions/_shared/schemas/dimensions.ts b/supabase/functions/_shared/schemas/dimensions.ts index 7c0afa870..29ebda2e1 100644 --- a/supabase/functions/_shared/schemas/dimensions.ts +++ b/supabase/functions/_shared/schemas/dimensions.ts @@ -53,6 +53,13 @@ export interface Dimension { */ export interface MemoryDimensions { tags: Dimension; + /** + * Labels the row must carry NONE of, ANDed with `tags` whatever its mode + * (migration 00114). A bare list, not a `Dimension`: its combination is fixed + * ("carries none of"), so a `mode` would be a field with one legal value. + * Empty means "not filtered", like every dimension's `values`. + */ + exclude_tags: string[]; source_agent: Dimension; trigger: Dimension; kind: Dimension; @@ -101,6 +108,7 @@ function digitsOnly(values: readonly string[]): string[] { export interface DimensionQueryParams { tags?: string | undefined; tags_mode?: TagsMode | undefined; + exclude_tags?: string | undefined; source_agent?: string | undefined; source_agent_mode?: ScalarFilterMode | undefined; trigger?: string | undefined; @@ -123,6 +131,7 @@ export interface DimensionQueryParams { export interface DimensionBodyFields { tags?: readonly string[] | undefined; tags_mode?: TagsMode | undefined; + exclude_tags?: readonly string[] | undefined; source_agent?: readonly string[] | undefined; source_agent_mode?: ScalarFilterMode | undefined; trigger?: readonly string[] | undefined; @@ -151,6 +160,7 @@ function build( }); return { tags: { values: values('tags'), mode: params.tags_mode ?? 'any' }, + exclude_tags: values('exclude_tags'), source_agent: scalar('source_agent'), trigger: scalar('trigger'), kind: scalar('kind'), diff --git a/supabase/functions/_shared/schemas/memory.ts b/supabase/functions/_shared/schemas/memory.ts index 8e802f654..5ff65b638 100644 --- a/supabase/functions/_shared/schemas/memory.ts +++ b/supabase/functions/_shared/schemas/memory.ts @@ -264,6 +264,16 @@ export const ListMemoriesQuerySchema = z.object({ key_prefix: z.string().min(1).max(512).optional(), tags: z.string().optional(), tags_mode: TagsModeSchema.optional().default('any'), + /** + * Labels the row must carry NONE of (comma-separated, like `tags`). + * + * An independent conjunct, ANDed with whatever `tags`/`tags_mode` ask — which + * is what makes "tagged `perf` AND NOT tagged `flaky`" expressible: one + * `tags` list has one mode, so it can include or exclude but never both. + * Same `not.ov` semantics as `tags_mode=none` (carries none of them), never + * the negation of containment. Migration 00114. + */ + exclude_tags: z.string().optional(), /** * Case-insensitive substring match against `key` OR `value`. * @@ -552,6 +562,8 @@ export const ListFacetsQuerySchema = z.object({ scope: RawScopeSchema.optional(), tags: z.string().optional(), tags_mode: TagsModeSchema.optional().default('any'), + /** Labels the row must carry NONE of — see `ListMemoriesQuerySchema.exclude_tags`. */ + exclude_tags: z.string().optional(), source_agent: ValueListSchema.optional(), source_agent_mode: ScalarFilterModeSchema.optional().default('in'), trigger: ValueListSchema.optional(), @@ -643,6 +655,8 @@ export const PivotQuerySchema = z.object({ scope: RawScopeSchema.optional(), tags: z.string().optional(), tags_mode: TagsModeSchema.optional().default('any'), + /** Labels the row must carry NONE of — see `ListMemoriesQuerySchema.exclude_tags`. */ + exclude_tags: z.string().optional(), source_agent: ValueListSchema.optional(), source_agent_mode: ScalarFilterModeSchema.optional().default('in'), trigger: ValueListSchema.optional(), @@ -727,6 +741,8 @@ export const ActivityQuerySchema = z.object({ scope: RawScopeSchema.optional(), tags: z.string().optional(), tags_mode: TagsModeSchema.optional().default('any'), + /** Labels the row must carry NONE of — see `ListMemoriesQuerySchema.exclude_tags`. */ + exclude_tags: z.string().optional(), source_agent: ValueListSchema.optional(), source_agent_mode: ScalarFilterModeSchema.optional().default('in'), trigger: ValueListSchema.optional(), @@ -1070,6 +1086,8 @@ const DimensionValuesSchema = z const dimensionBodyFields = { tags: DimensionValuesSchema.optional(), tags_mode: TagsModeSchema.optional().default('any'), + /** Labels the row must carry NONE of — see `ListMemoriesQuerySchema.exclude_tags`. */ + exclude_tags: DimensionValuesSchema.optional(), source_agent: DimensionValuesSchema.optional(), source_agent_mode: ScalarFilterModeSchema.optional().default('in'), trigger: DimensionValuesSchema.optional(), diff --git a/supabase/functions/memories/handlers/activity.ts b/supabase/functions/memories/handlers/activity.ts index a3c2fb69d..0ce9adb83 100644 --- a/supabase/functions/memories/handlers/activity.ts +++ b/supabase/functions/memories/handlers/activity.ts @@ -142,6 +142,7 @@ async function runActivity( p_scope: scopeFilter ?? null, p_tags: list(d.tags.values), p_tags_mode: d.tags.mode, + p_exclude_tags: list(d.exclude_tags), p_source_agent: list(d.source_agent.values), p_source_agent_mode: d.source_agent.mode, p_trigger: list(d.trigger.values), diff --git a/supabase/functions/memories/handlers/facets.ts b/supabase/functions/memories/handlers/facets.ts index a920b9cb5..16bd0b473 100644 --- a/supabase/functions/memories/handlers/facets.ts +++ b/supabase/functions/memories/handlers/facets.ts @@ -150,6 +150,7 @@ async function runFacets( p_scope: scopeFilter ?? null, p_tags: list(d.tags.values), p_tags_mode: d.tags.mode, + p_exclude_tags: list(d.exclude_tags), p_source_agent: list(d.source_agent.values), p_source_agent_mode: d.source_agent.mode, p_trigger: list(d.trigger.values), diff --git a/supabase/functions/memories/handlers/list.ts b/supabase/functions/memories/handlers/list.ts index b523134e2..86667458a 100644 --- a/supabase/functions/memories/handlers/list.ts +++ b/supabase/functions/memories/handlers/list.ts @@ -222,6 +222,7 @@ async function respondWithPage( // would turn into "matches nothing". p_tags: d.tags.values.length ? d.tags.values : null, p_tags_mode: d.tags.mode, + p_exclude_tags: d.exclude_tags.length ? d.exclude_tags : null, p_source_agent: d.source_agent.values.length ? d.source_agent.values : null, p_source_agent_mode: d.source_agent.mode, p_trigger: d.trigger.values.length ? d.trigger.values : null, diff --git a/supabase/functions/memories/handlers/pivot.ts b/supabase/functions/memories/handlers/pivot.ts index ff1b9f4fa..0cd40fc4f 100644 --- a/supabase/functions/memories/handlers/pivot.ts +++ b/supabase/functions/memories/handlers/pivot.ts @@ -134,6 +134,7 @@ async function runPivot( p_limit: limit + 1, p_tags: list(d.tags.values), p_tags_mode: d.tags.mode, + p_exclude_tags: list(d.exclude_tags), p_source_agent: list(d.source_agent.values), p_source_agent_mode: d.source_agent.mode, p_trigger: list(d.trigger.values), From 9782f2005a02fc680f92787c35acb5748e984b52 Mon Sep 17 00:00:00 2001 From: "mads.thines" Date: Tue, 20 Oct 2026 17:43:14 +0000 Subject: [PATCH 3/9] =?UTF-8?q?test(db):=20assert=20exclude=5Ftags=20compo?= =?UTF-8?q?ses=20on=20all=20four=20readers=20(=C2=A7114)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> --- supabase/tests/migrations.test.sql | 168 +++++++++++++++++++++++++++++ 1 file changed, 168 insertions(+) diff --git a/supabase/tests/migrations.test.sql b/supabase/tests/migrations.test.sql index d90099997..ba23a9fa1 100644 --- a/supabase/tests/migrations.test.sql +++ b/supabase/tests/migrations.test.sql @@ -44,6 +44,8 @@ values ('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()), + -- §114's own account, for §108's reason: exact counts out of four readers. + ('00000000-0000-0000-0000-000000000000', '00000000-0000-0000-0000-0000000000a8', 'authenticated', 'authenticated', 'lk-mig-a8@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()), @@ -10898,6 +10900,172 @@ begin end; $$; +-- ── 114. exclude_tags — "tagged a AND NOT tagged b" (00114) ───────────────── +-- One `tags` list has one mode, so a label filter could include OR exclude but +-- never both. 00114 appends `p_exclude_tags` to the four readers: an +-- always-applied "carries none of these" conjunct, independent of p_tags_mode. +-- +-- Five lessons on a fresh account, so every count is exact: +-- 114-perf {perf} host h1 +-- 114-perf-flaky {perf, flaky} host h1 +-- 114-flaky {flaky} host h2 +-- 114-bare {} host h2 +-- 114-perf-ci {perf, ci} host h2 +-- +-- AC-1: include + exclude compose on the list, under `all` AND `any`. +-- AC-2: a lone exclusion equals `tags_mode => 'none'` over the same list. +-- AC-3: facets apply it to every OTHER dimension, and the tag facet never +-- self-excludes it — an excluded label stays listed at count 0. +-- AC-4: activity and the pivot count the same rows the list returns, and a +-- tag-axis pivot yields no cell for an excluded label. +-- AC-5: a null exclusion is identical to omitting it. +-- AC-6: one overload per reader, each ending in `p_exclude_tags text[]`. +do $$ +declare + v_user uuid := '00000000-0000-0000-0000-0000000000a8'; + v_n bigint; + v_h1 bigint; + v_h2 bigint; + v_got text[]; + v_none text[]; +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, tags) values + (v_user, 'global', '114-perf', 'v', 'lesson', 'h1', array['perf']), + (v_user, 'global', '114-perf-flaky', 'v', 'lesson', 'h1', array['perf', 'flaky']), + (v_user, 'global', '114-flaky', 'v', 'lesson', 'h2', array['flaky']), + (v_user, 'global', '114-bare', 'v', 'lesson', 'h2', array[]::text[]), + (v_user, 'global', '114-perf-ci', 'v', 'lesson', 'h2', array['perf', 'ci']); + + -- ── AC-1: include + exclude on the list ───────────────────────────────── + select array_agg(key order by key) into v_got from lorekit_memory_list( + p_user_id => v_user, p_scope => 'global', + p_tags => array['perf'], p_tags_mode => 'all', p_exclude_tags => array['flaky'], + p_limit => 100); + assert v_got = array['114-perf', '114-perf-ci'], + format('114 AC-1a: perf AND NOT flaky must list perf and perf-ci, got %s', v_got); + + select array_agg(key order by key) into v_got from lorekit_memory_list( + p_user_id => v_user, p_scope => 'global', + p_tags => array['ci', 'flaky'], p_tags_mode => 'any', p_exclude_tags => array['perf'], + p_limit => 100); + assert v_got = array['114-flaky'], + format('114 AC-1b: (ci OR flaky) AND NOT perf must list only flaky, got %s', v_got); + + -- Several excluded labels: none of them, never "not all of them". + select array_agg(key order by key) into v_got from lorekit_memory_list( + p_user_id => v_user, p_scope => 'global', + p_exclude_tags => array['flaky', 'ci'], p_limit => 100); + assert v_got = array['114-bare', '114-perf'], + format('114 AC-1c: NOT flaky AND NOT ci must list bare and perf, got %s', v_got); + + -- ── AC-2: a lone exclusion equals tags_mode none ─────────────────────── + select array_agg(key order by key) into v_got from lorekit_memory_list( + p_user_id => v_user, p_scope => 'global', p_exclude_tags => array['flaky'], p_limit => 100); + select array_agg(key order by key) into v_none from lorekit_memory_list( + p_user_id => v_user, p_scope => 'global', + p_tags => array['flaky'], p_tags_mode => 'none', p_limit => 100); + assert v_got = v_none and v_got = array['114-bare', '114-perf', '114-perf-ci'], + format('114 AC-2: exclude_tags => {flaky} must equal tags_mode none, got %s vs %s', v_got, v_none); + + -- ── AC-3: facets ──────────────────────────────────────────────────────── + select count into v_h1 from lorekit_memory_facets( + p_user_id => v_user, p_scope => 'global', + p_tags => array['perf'], p_tags_mode => 'all', p_exclude_tags => array['flaky']) + where facet = 'host' and value = 'h1'; + select count into v_h2 from lorekit_memory_facets( + p_user_id => v_user, p_scope => 'global', + p_tags => array['perf'], p_tags_mode => 'all', p_exclude_tags => array['flaky']) + where facet = 'host' and value = 'h2'; + assert v_h1 = 1 and v_h2 = 1, + format('114 AC-3a: the host facet must count the rows the list returns (h1=1, h2=1), ' + 'got h1=%s h2=%s', v_h1, v_h2); + + -- `any` mode: the tag facet self-excludes the INCLUDE condition, so perf and + -- ci show what selecting them yields — still without the excluded label. + select count into v_n from lorekit_memory_facets( + p_user_id => v_user, p_scope => 'global', + p_tags => array['ci'], p_tags_mode => 'any', p_exclude_tags => array['flaky']) + where facet = 'tag' and value = 'perf'; + assert v_n = 2, + format('114 AC-3b: tag perf must count perf and perf-ci (perf-flaky is excluded), got %s', v_n); + + -- `all` mode: co-occurrence folds the include back in, the exclusion stays. + select count into v_n from lorekit_memory_facets( + p_user_id => v_user, p_scope => 'global', + p_tags => array['perf'], p_tags_mode => 'all', p_exclude_tags => array['flaky']) + where facet = 'tag' and value = 'ci'; + assert v_n = 1, format('114 AC-3c: tag ci must count perf-ci only, got %s', v_n); + + -- The excluded label stays in the catalog at zero, so the menu can offer to + -- take the exclusion back off. + select count into v_n from lorekit_memory_facets( + p_user_id => v_user, p_scope => 'global', + p_tags => array['perf'], p_tags_mode => 'all', p_exclude_tags => array['flaky']) + where facet = 'tag' and value = 'flaky'; + assert v_n = 0, format('114 AC-3d: an excluded label must be listed at count 0, got %s', v_n); + assert exists ( + select 1 from lorekit_memory_facets( + p_user_id => v_user, p_scope => 'global', + p_tags => array['perf'], p_tags_mode => 'all', p_exclude_tags => array['flaky']) + where facet = 'tag' and value = 'flaky'), + '114 AC-3e: an excluded label must NOT disappear from the tag facet'; + + -- ── AC-4: activity and the pivot ──────────────────────────────────────── + select coalesce(sum(count), 0) into v_n from lorekit_memory_activity( + p_user_id => v_user, p_scope => 'global', + p_tags => array['perf'], p_tags_mode => 'all', p_exclude_tags => array['flaky']); + assert v_n = 2, format('114 AC-4a: activity must count the same 2 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_tags => array['perf'], p_tags_mode => 'all', p_exclude_tags => array['flaky']); + assert v_n = 2, format('114 AC-4b: the pivot must count the same 2 rows, got %s', v_n); + + select count(*) into v_n from lorekit_memory_pivot('tag', 'host', + p_user_id => v_user, p_scope => 'global', p_exclude_tags => array['flaky']) + where row_value = 'flaky'; + assert v_n = 0, format('114 AC-4c: a tag-axis pivot must yield no cell for an excluded label, got %s', v_n); + + -- ── AC-5: null is a no-op ─────────────────────────────────────────────── + select count(*) into v_n from lorekit_memory_list( + p_user_id => v_user, p_scope => 'global', p_exclude_tags => null, p_limit => 100); + assert v_n = 5, format('114 AC-5a: a null exclusion must change nothing on the list, got %s', v_n); + + select count into v_n from lorekit_memory_facets( + p_user_id => v_user, p_scope => 'global', p_exclude_tags => null) + where facet = 'tag' and value = 'flaky'; + assert v_n = 2, format('114 AC-5b: a null exclusion must change nothing on facets, got %s', v_n); + + reset role; + perform set_config('request.jwt.claims', '', true); +end; +$$; + +-- AC-6: the catalog side. +do $$ +declare + r record; +begin + for r in + select p.proname, count(*) over (partition by p.proname) as overloads, + pg_get_function_identity_arguments(p.oid) as args + 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') + loop + assert r.overloads = 1, + format('114 AC-6a: %s must have exactly one overload, found %s', r.proname, r.overloads); + assert r.args like '%p_exclude_tags text[]', + format('114 AC-6b: %s must end in p_exclude_tags text[], got (%s)', r.proname, r.args); + end loop; +end; +$$; + rollback; \echo 'migrations.test.sql: all assertions passed' From 00415bcc4db03de4260e72e29b454e9658d829dd Mon Sep 17 00:00:00 2001 From: "mads.thines" Date: Tue, 20 Oct 2026 17:47:40 +0000 Subject: [PATCH 4/9] feat(web): tag include + exclude slots and filter in/out logic in the filter model The tag dimension keeps an include pill and an exclude pill that both hold (exclude_tags on the wire). applyValueFilter is the panel's filter in / filter out: the bar ANDed with the click, simplified exactly for single-valued fields. The policy handoff is withheld when a policy could not express it. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> --- .../web/src/components/lore/FilterBar.tsx | 25 +- .../web/src/components/lore/LoreExplorer.tsx | 17 +- .../settings/GroomingRuleBuilder.tsx | 6 +- packages/web/src/lib/filters-polarity.spec.ts | 230 ++++++++++++++ packages/web/src/lib/filters.spec.ts | 20 +- packages/web/src/lib/filters.ts | 280 ++++++++++++++++-- 6 files changed, 538 insertions(+), 40 deletions(-) create mode 100644 packages/web/src/lib/filters-polarity.spec.ts diff --git a/packages/web/src/components/lore/FilterBar.tsx b/packages/web/src/components/lore/FilterBar.tsx index c2c87fbc8..4bf8a1092 100644 --- a/packages/web/src/components/lore/FilterBar.tsx +++ b/packages/web/src/components/lore/FilterBar.tsx @@ -42,7 +42,16 @@ import { Archive } from 'lucide-react'; import { FilterMenu } from './FilterMenu'; import { FilterPill } from './FilterPill'; import { RetentionPill } from './RetentionPill'; -import { filtersPhrase, type FacetValue, type Filter, type FilterField, type FilterOperator } from '@/lib/filters'; +import { + filterKey, + filterPolarity, + filtersPhrase, + type FacetValue, + type Filter, + type FilterField, + type FilterOperator, + type FilterPolarity, +} from '@/lib/filters'; import { NO_RETENTION_CONDITIONS, RETENTION_FIELDS, @@ -58,8 +67,12 @@ interface FilterBarProps { facets: FacetValue[]; filters: Filter[]; onToggleValue: (field: FilterField, value: string) => void; - onOperatorChange: (field: FilterField, operator: FilterOperator) => void; - onRemove: (field: FilterField) => void; + /** + * `polarity` names WHICH pill when a field has two (a tag include and a tag + * exclude); a caller whose bar can never hold both may ignore it. + */ + onOperatorChange: (field: FilterField, operator: FilterOperator, polarity: FilterPolarity) => void; + onRemove: (field: FilterField, polarity: FilterPolarity) => void; onClearAll: () => void; /** Set by a pill's value segment to reopen the menu at that dimension. */ editingField: FilterField | null; @@ -194,11 +207,11 @@ export function FilterPillRow({ {filters.map((filter) => ( onOperatorChange(filter.field, op)} + onOperatorChange={(op) => onOperatorChange(filter.field, op, filterPolarity(filter))} onEditValues={() => onEditField(filter.field)} - onRemove={() => onRemove(filter.field)} + onRemove={() => onRemove(filter.field, filterPolarity(filter))} /> ))} {retentionPills.map(({ field, value }) => ( diff --git a/packages/web/src/components/lore/LoreExplorer.tsx b/packages/web/src/components/lore/LoreExplorer.tsx index 2bdb24dad..786e69065 100644 --- a/packages/web/src/components/lore/LoreExplorer.tsx +++ b/packages/web/src/components/lore/LoreExplorer.tsx @@ -135,6 +135,7 @@ import { usePersistedPreference } from '@/lib/hooks/usePersistedPreference'; import type { DuplicateCluster } from '@lorekit/schemas/memory'; import { filtersParamValue, + hasExplorerOnlyFilters, removeFilter, resolveFilters, setFilterOperator, @@ -143,6 +144,7 @@ import { type Filter, type FilterField, type FilterOperator, + type FilterPolarity, } from '@/lib/filters'; import { FilterMenuTrigger, FilterPillRow } from './FilterBar'; import { SortSelect } from './SortSelect'; @@ -563,9 +565,12 @@ export function LoreExplorer({ scopes, heatmapData }: LoreExplorerProps) { ); // "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. + // otherwise), no Explorer-only reverse direction is set, and no tag is both + // included and excluded (a policy has one `tags` condition — 00114). const canCreatePolicy = - retentionPoliciesEnabled && !hasExplorerOnlyConditions(retentionConditions); + retentionPoliciesEnabled && + !hasExplorerOnlyConditions(retentionConditions) && + !hasExplorerOnlyFilters(filters); // 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); @@ -971,8 +976,8 @@ export function LoreExplorer({ scopes, heatmapData }: LoreExplorerProps) { setSelectedCluster(null); } - function handleOperatorChange(field: FilterField, operator: FilterOperator) { - const next = setFilterOperator(filters, field, operator); + function handleOperatorChange(field: FilterField, operator: FilterOperator, polarity: FilterPolarity) { + const next = setFilterOperator(filters, field, operator, polarity); setFilters(next); track({ name: 'lore.filter_changed', @@ -985,8 +990,8 @@ export function LoreExplorer({ scopes, heatmapData }: LoreExplorerProps) { setSelectedCluster(null); } - function handleRemoveFilter(field: FilterField) { - const next = removeFilter(filters, field); + function handleRemoveFilter(field: FilterField, polarity: FilterPolarity) { + const next = removeFilter(filters, field, polarity); setFilters(next); track({ name: 'lore.filter_changed', action: 'remove', field, ...filterCounts(next) }); closeLesson('filter-change'); diff --git a/packages/web/src/components/settings/GroomingRuleBuilder.tsx b/packages/web/src/components/settings/GroomingRuleBuilder.tsx index fe3ecc4ec..686e555f9 100644 --- a/packages/web/src/components/settings/GroomingRuleBuilder.tsx +++ b/packages/web/src/components/settings/GroomingRuleBuilder.tsx @@ -63,6 +63,7 @@ import { } from '@/lib/retention-filter'; import { normalizeFilters, + policyExpressibleFilters, removeFilter, setFilterOperator, toggleFilterValue, @@ -134,7 +135,10 @@ function conditionsFromPrefillParams(params: URLSearchParams): Conditions | null let filters: Filter[] = []; if (rawFilters) { try { - filters = normalizeFilters(JSON.parse(rawFilters)); + // A policy has one `tags` condition, so a tag include + exclude (which the + // Explorer withholds the handoff for) can only arrive by a hand-edited + // link — keep only what the saved policy will actually match. + filters = policyExpressibleFilters(normalizeFilters(JSON.parse(rawFilters))); } catch { filters = []; } diff --git a/packages/web/src/lib/filters-polarity.spec.ts b/packages/web/src/lib/filters-polarity.spec.ts new file mode 100644 index 000000000..76fce2279 --- /dev/null +++ b/packages/web/src/lib/filters-polarity.spec.ts @@ -0,0 +1,230 @@ +/** + * Filter in / filter out — the polarity half of the filter bar's model. + * + * What these pin is the boolean logic: every click must produce the existing + * bar ANDed with "is / is not this value", expressed in the bar's own shape — + * OR within a dimension, AND across dimensions, and for the multi-valued tag + * dimension an include slot and an exclude slot that hold at the same time. + */ + +import { describe, it, expect } from 'vitest'; +import { ListMemoriesBodySchema, ListMemoriesQuerySchema } from '@lorekit/schemas/memory'; +import { + applyValueFilter, + filterKey, + filtersToBody, + filtersToQueryParams, + findFilter, + hasExplorerOnlyFilters, + normalizeFilters, + policyExpressibleFilters, + removeFilter, + selectedValues, + setFilterOperator, + toggleFilterValue, + valuePolarity, + type Filter, +} from './filters'; + +const tagIn = (...values: string[]): Filter => ({ field: 'label', operator: 'all', values }); +const tagAny = (...values: string[]): Filter => ({ field: 'label', operator: 'in', values }); +const tagOut = (...values: string[]): Filter => ({ field: 'label', operator: 'nin', values }); +const hostIs = (...values: string[]): Filter => ({ field: 'host', operator: 'in', values }); +const hostNot = (...values: string[]): Filter => ({ field: 'host', operator: 'nin', values }); + +describe('normalizeFilters — the tag dimension keeps two slots', () => { + it('keeps a tag include and a tag exclude side by side, include first', () => { + expect(normalizeFilters([tagOut('flaky'), tagIn('perf')])).toEqual([tagIn('perf'), tagOut('flaky')]); + }); + + it('still merges two conditions of the SAME slot', () => { + expect(normalizeFilters([tagOut('a'), tagOut('b')])).toEqual([tagOut('a', 'b')]); + expect(normalizeFilters([tagIn('a'), tagAny('b')])).toEqual([tagIn('a', 'b')]); + }); + + it('keeps one condition per scalar field whatever the polarity', () => { + expect(normalizeFilters([hostIs('a'), hostNot('b')])).toEqual([hostIs('a', 'b')]); + }); + + it('gives each pill its own key', () => { + expect(filterKey(tagIn('a'))).not.toBe(filterKey(tagOut('b'))); + expect(filterKey(hostIs('a'))).toBe(filterKey(hostNot('b'))); + }); +}); + +describe('applyValueFilter — tags (independent include and exclude slots)', () => { + it('filter out on an empty bar excludes the tag', () => { + expect(applyValueFilter([], 'label', 'flaky', 'exclude')).toEqual([tagOut('flaky')]); + }); + + it('filter in on an empty bar starts an "includes all" condition', () => { + expect(applyValueFilter([], 'label', 'perf', 'include')).toEqual([tagIn('perf')]); + }); + + it('perf AND NOT flaky: an exclusion composes with an existing include', () => { + expect(applyValueFilter([tagIn('perf')], 'label', 'flaky', 'exclude')).toEqual([ + tagIn('perf'), + tagOut('flaky'), + ]); + }); + + it('an include composes with an existing exclusion instead of joining it', () => { + // The old single-slot model could only grow the exclusion here. + expect(applyValueFilter([tagOut('flaky')], 'label', 'perf', 'include')).toEqual([ + tagIn('perf'), + tagOut('flaky'), + ]); + }); + + it('several exclusions are NOT a AND NOT b — one "includes none" pill', () => { + expect(applyValueFilter([tagOut('a')], 'label', 'b', 'exclude')).toEqual([tagOut('a', 'b')]); + }); + + it('a second include follows the include pill: AND under "all", OR under "any"', () => { + expect(applyValueFilter([tagIn('a')], 'label', 'b', 'include')).toEqual([tagIn('a', 'b')]); + expect(applyValueFilter([tagAny('a')], 'label', 'b', 'include')).toEqual([tagAny('a', 'b')]); + }); + + it('excluding a required tag MOVES it — never both required and forbidden', () => { + expect(applyValueFilter([tagIn('a', 'b')], 'label', 'b', 'exclude')).toEqual([tagIn('a'), tagOut('b')]); + expect(applyValueFilter([tagIn('b')], 'label', 'b', 'exclude')).toEqual([tagOut('b')]); + }); + + it('including an excluded tag moves it back', () => { + expect(applyValueFilter([tagIn('a'), tagOut('b')], 'label', 'b', 'include')).toEqual([tagIn('a', 'b')]); + }); + + it('pressing the same polarity again takes it off', () => { + expect(applyValueFilter([tagIn('perf'), tagOut('flaky')], 'label', 'flaky', 'exclude')).toEqual([tagIn('perf')]); + expect(applyValueFilter([tagIn('perf'), tagOut('flaky')], 'label', 'perf', 'include')).toEqual([tagOut('flaky')]); + }); +}); + +describe('applyValueFilter — scalar fields (one exact condition)', () => { + it('filter out on an empty bar: is not X', () => { + expect(applyValueFilter([], 'host', 'aw', 'exclude')).toEqual([hostNot('aw')]); + }); + + it('is not A, then filter out X: is not A AND is not X = neither of A, X', () => { + expect(applyValueFilter([hostNot('a')], 'host', 'x', 'exclude')).toEqual([hostNot('a', 'x')]); + }); + + it('is A or X, then filter out X: exactly "is A"', () => { + expect(applyValueFilter([hostIs('a', 'x')], 'host', 'x', 'exclude')).toEqual([hostIs('a')]); + }); + + it('is X, then filter out X: flips to "is not X" rather than matching nothing', () => { + expect(applyValueFilter([hostIs('x')], 'host', 'x', 'exclude')).toEqual([hostNot('x')]); + }); + + it('is A, then filter out X: unchanged — "is A" already rules X out', () => { + expect(applyValueFilter([hostIs('a')], 'host', 'x', 'exclude')).toEqual([hostIs('a')]); + }); + + it('is A, then filter in X: OR within the dimension', () => { + expect(applyValueFilter([hostIs('a')], 'host', 'x', 'include')).toEqual([hostIs('a', 'x')]); + }); + + it('is not A, then filter in X: exactly "is X"', () => { + expect(applyValueFilter([hostNot('a')], 'host', 'x', 'include')).toEqual([hostIs('x')]); + expect(applyValueFilter([hostNot('a', 'x')], 'host', 'x', 'include')).toEqual([hostIs('x')]); + }); + + it('pressing the same polarity again takes it off', () => { + expect(applyValueFilter([hostIs('a', 'x')], 'host', 'x', 'include')).toEqual([hostIs('a')]); + expect(applyValueFilter([hostNot('x')], 'host', 'x', 'exclude')).toEqual([]); + }); + + it('leaves every other dimension alone (AND across dimensions)', () => { + expect(applyValueFilter([tagIn('perf'), hostIs('a')], 'kind', 'bus', 'exclude')).toEqual([ + tagIn('perf'), + { field: 'kind', operator: 'nin', values: ['bus'] }, + hostIs('a'), + ]); + }); +}); + +describe('reading both slots', () => { + const bar = [tagIn('perf'), tagOut('flaky')]; + + it('valuePolarity names the slot holding a value', () => { + expect(valuePolarity(bar, 'label', 'perf')).toBe('include'); + expect(valuePolarity(bar, 'label', 'flaky')).toBe('exclude'); + expect(valuePolarity(bar, 'label', 'ci')).toBeNull(); + expect(valuePolarity([hostIs('a')], 'host', 'x')).toBeNull(); + }); + + it('selectedValues covers both slots, so the menu can uncheck an exclusion', () => { + expect(selectedValues(bar, 'label')).toEqual(['perf', 'flaky']); + expect(toggleFilterValue(bar, 'label', 'flaky')).toEqual([tagIn('perf')]); + }); + + it('a menu-checked value joins the include slot when there is one', () => { + expect(toggleFilterValue(bar, 'label', 'ci')).toEqual([tagIn('perf', 'ci'), tagOut('flaky')]); + }); + + it('an exclude-only tag filter still grows its exclusion from the menu, as before', () => { + expect(toggleFilterValue([tagOut('flaky')], 'label', 'ci')).toEqual([tagOut('flaky', 'ci')]); + }); +}); + +describe('setFilterOperator / removeFilter with two tag pills', () => { + const bar = [tagIn('perf'), tagOut('flaky')]; + + it('changes only the pill it is given', () => { + expect(setFilterOperator(bar, 'label', 'in', 'include')).toEqual([tagAny('perf'), tagOut('flaky')]); + }); + + it('moving a pill onto the other slot merges, and the chosen operator wins', () => { + expect(setFilterOperator(bar, 'label', 'nin', 'include')).toEqual([tagOut('perf', 'flaky')]); + expect(setFilterOperator(bar, 'label', 'in', 'exclude')).toEqual([tagAny('flaky', 'perf')]); + }); + + it('removes one pill, or with no polarity every pill of the field', () => { + expect(removeFilter(bar, 'label', 'exclude')).toEqual([tagIn('perf')]); + expect(removeFilter(bar, 'label', 'include')).toEqual([tagOut('flaky')]); + expect(removeFilter(bar, 'label')).toEqual([]); + }); + + it('findFilter reads a slot by polarity', () => { + expect(findFilter(bar, 'label', 'exclude')).toEqual(tagOut('flaky')); + expect(findFilter(bar, 'label')).toEqual(tagIn('perf')); + }); +}); + +describe('the wire: exclude_tags only beside an include', () => { + it('include + exclude travel as tags + exclude_tags, over both transports', () => { + const bar = [tagIn('perf'), tagOut('flaky', 'ci')]; + const body = filtersToBody(bar); + expect(body).toEqual({ tags: ['perf'], tags_mode: 'all', exclude_tags: ['flaky', 'ci'] }); + expect(ListMemoriesBodySchema.safeParse(body).success).toBe(true); + + const query = filtersToQueryParams(bar); + expect(query).toEqual({ tags: 'perf', tags_mode: 'all', exclude_tags: 'flaky,ci' }); + expect(ListMemoriesQuerySchema.safeParse(query).success).toBe(true); + }); + + it('a lone exclusion keeps its historical encoding', () => { + expect(filtersToBody([tagOut('flaky')])).toEqual({ tags: ['flaky'], tags_mode: 'none' }); + }); + + it('a scalar exclusion is its own *_mode, as always', () => { + expect(filtersToBody([hostNot('aw')])).toEqual({ host: ['aw'], host_mode: 'nin' }); + }); +}); + +describe('retention policies cannot express include + exclude on tags', () => { + it('flags the combination, and only the combination', () => { + expect(hasExplorerOnlyFilters([tagIn('perf'), tagOut('flaky')])).toBe(true); + expect(hasExplorerOnlyFilters([tagOut('flaky')])).toBe(false); + expect(hasExplorerOnlyFilters([tagIn('perf'), hostNot('aw')])).toBe(false); + }); + + it('a policy prefill keeps what a policy can match and drops the rest visibly', () => { + expect(policyExpressibleFilters([tagIn('perf'), tagOut('flaky'), hostNot('aw')])).toEqual([ + tagIn('perf'), + hostNot('aw'), + ]); + expect(policyExpressibleFilters([tagOut('flaky')])).toEqual([tagOut('flaky')]); + }); +}); diff --git a/packages/web/src/lib/filters.spec.ts b/packages/web/src/lib/filters.spec.ts index e675fe73e..6c4f26213 100644 --- a/packages/web/src/lib/filters.spec.ts +++ b/packages/web/src/lib/filters.spec.ts @@ -87,14 +87,32 @@ describe('normalizeFilters', () => { }); it('merges a duplicated field into one filter rather than dropping half of it', () => { + expect( + normalizeFilters([ + { field: 'host', operator: 'in', values: ['aw'] }, + { field: 'host', operator: 'nin', values: ['reviewer'] }, + ]), + ).toEqual([{ field: 'host', operator: 'in', values: ['aw', 'reviewer'] }]); expect( normalizeFilters([ { field: 'label', operator: 'in', values: ['perf'] }, - { field: 'label', operator: 'nin', values: ['ci'] }, + { field: 'label', operator: 'all', values: ['ci'] }, ]), ).toEqual([{ field: 'label', operator: 'in', values: ['perf', 'ci'] }]); }); + it('keeps a tag include and a tag exclude as two conditions — both hold (00114)', () => { + expect( + normalizeFilters([ + { field: 'label', operator: 'in', values: ['perf'] }, + { field: 'label', operator: 'nin', values: ['ci'] }, + ]), + ).toEqual([ + { field: 'label', operator: 'in', values: ['perf'] }, + { field: 'label', operator: 'nin', values: ['ci'] }, + ]); + }); + it('emits in FILTER_FIELDS order so a shared link renders identically', () => { const out = normalizeFilters([ { field: 'pr', operator: 'in', values: ['482'] }, diff --git a/packages/web/src/lib/filters.ts b/packages/web/src/lib/filters.ts index b35f26bab..d6c0ff0ff 100644 --- a/packages/web/src/lib/filters.ts +++ b/packages/web/src/lib/filters.ts @@ -16,6 +16,13 @@ * precedence grammar. Anything richer belongs in `POST /memories/search`'s * filter tree. * + * The one exception to "one pill per dimension" is a MULTI-VALUED dimension + * (`label`): a row carries many labels, so "includes perf" and "includes none + * of flaky" are independent conditions that can both hold, and the bar keeps + * them as two pills — an include slot and an exclude slot — ANDed together + * (`exclude_tags`, migration 00114). For a single-valued column the two + * collapse exactly into one condition, so every other dimension keeps one pill. + * * Kept dependency-free (no React, no network) so every decision the bar makes * is unit-testable in the node vitest project, mirroring `tag-filter.ts` and * `org-ui.ts`. The impure shells are `components/lore/FilterMenu.tsx` (the @@ -80,6 +87,17 @@ export interface Filter { values: string[]; } +/** + * Whether a condition keeps rows that match its values (`include`) or rows + * that do not (`exclude` — the `nin` operator). The detail panel's "filter in" + * and "filter out" buttons are the two polarities. + */ +export type FilterPolarity = 'include' | 'exclude'; + +export function filterPolarity(filter: Pick): FilterPolarity { + return filter.operator === 'nin' ? 'exclude' : 'include'; +} + /** * The `GET /memories/facets` dimension a field's values are catalogued under. * @@ -124,6 +142,14 @@ export interface FilterFieldDescriptor { operators: readonly FilterOperator[]; /** How a raw value is rendered (a PR number reads as `#482`, not `482`). */ format: (value: string) => string; + /** + * A row carries MANY values of this dimension (only `label`). Including some + * values and excluding others are then independent conditions that can both + * hold, so the bar keeps one include slot AND one exclude slot for it. For a + * single-valued column "is A or B" and "is not X" collapse into one exact + * condition, so it never needs a second slot. + */ + multiValued?: boolean; } /** @@ -157,6 +183,7 @@ export const FILTER_FIELDS: readonly FilterFieldDescriptor[] = [ facet: 'tag', operators: ['all', 'in', 'nin'], format: (v) => v, + multiValued: true, }, { // The coarsest partition of the store, so it sits high: `bus` events are @@ -272,9 +299,11 @@ export function requireField(field: FilterField): FilterFieldDescriptor { * malformed URL. The invariants it establishes, which every consumer below may * then assume: * - * - at most one filter per field (a second one for the same field is merged - * into the first, because two pills of one dimension have no rendering in a - * flat bar and no meaning the operator cannot already express); + * - at most one filter per field — or, for a {@link FilterFieldDescriptor.multiValued} + * field, at most one per polarity (an include slot and an exclude slot). A + * second filter for the same slot is merged into the first, because two pills + * of one condition have no rendering in a flat bar and no meaning the + * operator cannot already express; * - values are trimmed, de-duplicated and non-empty; * - a filter with no values is removed entirely — an empty pill is a control * that filters nothing and cannot be cleared by the thing it looks like; @@ -283,7 +312,7 @@ export function requireField(field: FilterField): FilterFieldDescriptor { export function normalizeFilters(raw: unknown): Filter[] { if (!Array.isArray(raw)) return []; - const byField = new Map(); + const bySlot = new Map(); for (const candidate of raw) { if (!candidate || typeof candidate !== 'object') continue; @@ -299,20 +328,37 @@ export function normalizeFilters(raw: unknown): Filter[] { ? (operator as FilterOperator) : (descriptor.operators[0] as FilterOperator); - const existing = byField.get(descriptor.field); + const slot = slotKey(descriptor.field, op); + const existing = bySlot.get(slot); if (existing) { // Merge rather than drop: a duplicated field in a hand-edited URL most // likely means "all of these values", and losing half of them silently // is the worse failure. existing.values = normalizeTagList([...existing.values, ...cleanValues]); } else { - byField.set(descriptor.field, { field: descriptor.field, operator: op, values: cleanValues }); + bySlot.set(slot, { field: descriptor.field, operator: op, values: cleanValues }); } } - // Emit in FILTER_FIELDS order so the pill row is stable across reloads and - // shared links, rather than reflecting the order values happened to be added. - return FILTER_FIELDS.map((d) => byField.get(d.field)).filter((f): f is Filter => f !== undefined); + // Emit in FILTER_FIELDS order — include slot before exclude slot — so the + // pill row is stable across reloads and shared links, rather than reflecting + // the order values happened to be added. + return FILTER_FIELDS.flatMap((d) => + [bySlot.get(d.field), bySlot.get(`${d.field}:exclude`)].filter((f): f is Filter => f !== undefined), + ); +} + +/** + * The slot a condition occupies: the field itself, or `:exclude` for + * the exclude slot of a multi-valued field. Two conditions in one slot merge. + */ +function slotKey(field: FilterField, operator: FilterOperator): string { + return requireField(field).multiValued && operator === 'nin' ? `${field}:exclude` : field; +} + +/** A stable identity for a committed condition — one pill, one key. */ +export function filterKey(filter: Pick): string { + return slotKey(filter.field, filter.operator); } /** @@ -403,13 +449,44 @@ export function filtersParamValue( // ── Reading ────────────────────────────────────────────────────────────────── -export function findFilter(filters: readonly Filter[], field: FilterField): Filter | undefined { - return filters.find((f) => f.field === field); +/** + * The field's condition — or, given a polarity, the condition of that polarity + * (a multi-valued field can hold one of each). + */ +export function findFilter( + filters: readonly Filter[], + field: FilterField, + polarity?: FilterPolarity, +): Filter | undefined { + return filters.find( + (f) => f.field === field && (polarity === undefined || filterPolarity(f) === polarity), + ); } -/** The values currently selected for a field. Empty when the field has no filter. */ +/** + * The values currently named for a field, across BOTH slots of a multi-valued + * field — so the menu shows an excluded value as checked, and unchecking it + * takes the exclusion off. Empty when the field has no filter. + */ export function selectedValues(filters: readonly Filter[], field: FilterField): string[] { - return findFilter(filters, field)?.values ?? []; + return filters.filter((f) => f.field === field).flatMap((f) => f.values); +} + +/** + * Whether a value is currently included or excluded for its field, or `null` + * when no condition names it. A scalar `is` condition names its values as + * included and an `is not` one as excluded; a value it does not name is + * `null` even when the condition happens to rule it out, because nothing on + * screen says so. + */ +export function valuePolarity( + filters: readonly Filter[], + field: FilterField, + value: string, +): FilterPolarity | null { + const trimmed = value.trim(); + const holder = filters.find((f) => f.field === field && f.values.includes(trimmed)); + return holder ? filterPolarity(holder) : null; } export function isValueSelected( @@ -443,37 +520,162 @@ export function toggleFilterValue( const trimmed = value.trim(); if (!trimmed) return normalizeFilters(filters as unknown[]); - const current = findFilter(filters, field); - if (!current) { + if (valuePolarity(filters, field, trimmed) !== null) return withoutValue(filters, field, trimmed); + + // A new value joins the include slot when there is one, else the field's + // only condition (an exclude-only tag filter grows its exclusion, as it + // always has), else starts a condition with the field's default operator. + const target = findFilter(filters, field, 'include') ?? findFilter(filters, field); + if (!target) { return normalizeFilters([ ...filters, { field, operator: requireField(field).operators[0], values: [trimmed] }, ]); } + return withValueAdded(filters, target, trimmed); +} + +/** + * The detail panel's "filter in" / "filter out" for one value — and, pressed + * again while that value is already included / excluded, the way back off. + * + * Every result is the existing bar ANDed with the click, expressed in the + * bar's own shape: + * + * - **Exclude, value not yet named.** `bar AND NOT value`. A tag joins the + * exclude slot ("includes none of a, b" = NOT a AND NOT b); a scalar `is not` + * grows the same way; a scalar `is` that already rules the value out is + * unchanged, because `(is A) AND (is not X)` is just `is A`. + * - **Include, value not yet named.** Joins the include slot — OR within the + * dimension for `is either of` / `includes any`, AND for `includes all`, + * which is what every other way of adding a value to that pill means. A + * scalar `is not` is replaced by `is value`: for a single-valued column, + * `(is not A) AND (is X)` is exactly `is X`. + * - **The value currently has the OTHER polarity.** It moves: a value can never + * be both required and forbidden (that is the empty set, which no click asks + * for). Excluding a scalar's ONLY included value therefore flips `is X` to + * `is not X`, and excluding a required tag moves it to the exclude slot. + */ +export function applyValueFilter( + filters: readonly Filter[], + field: FilterField, + value: string, + polarity: FilterPolarity, +): Filter[] { + const trimmed = value.trim(); + if (!trimmed) return normalizeFilters(filters as unknown[]); + + const current = valuePolarity(filters, field, trimmed); + if (current === polarity) return withoutValue(filters, field, trimmed); + + if (requireField(field).multiValued) { + // Independent slots: take the value out of the other one (if it was there), + // then add it to this one. + const rest = withoutValue(filters, field, trimmed); + const slot = findFilter(rest, field, polarity); + if (slot) return withValueAdded(rest, slot, trimmed); + const operator = polarity === 'exclude' ? 'nin' : requireField(field).operators[0]; + return normalizeFilters([...rest, { field, operator, values: [trimmed] }]); + } - const nextValues = current.values.includes(trimmed) - ? current.values.filter((v) => v !== trimmed) - : [...current.values, trimmed]; + // Single-valued: one condition per field, simplified exactly. + const condition = findFilter(filters, field); + if (!condition) { + return normalizeFilters([ + ...filters, + { field, operator: polarity === 'exclude' ? 'nin' : 'in', values: [trimmed] }, + ]); + } + if (polarity === 'include') { + return condition.operator === 'in' + ? withValueAdded(filters, condition, trimmed) + : replaceCondition(filters, condition, { field, operator: 'in', values: [trimmed] }); + } + if (condition.operator === 'nin') return withValueAdded(filters, condition, trimmed); + if (current === null) return normalizeFilters(filters as unknown[]); + const remaining = condition.values.filter((v) => v !== trimmed); + return remaining.length > 0 + ? replaceCondition(filters, condition, { ...condition, values: remaining }) + : replaceCondition(filters, condition, { field, operator: 'nin', values: [trimmed] }); +} +/** Remove a value from whichever condition names it; a condition left empty goes. */ +function withoutValue(filters: readonly Filter[], field: FilterField, value: string): Filter[] { return normalizeFilters( - filters.map((f) => (f.field === field ? { ...f, values: nextValues } : f)), + filters.map((f) => (f.field === field ? { ...f, values: f.values.filter((v) => v !== value) } : f)), ); } -/** Replace a field's operator. A no-op for an operator the field does not offer. */ +function withValueAdded(filters: readonly Filter[], target: Filter, value: string): Filter[] { + return replaceCondition(filters, target, { ...target, values: [...target.values, value] }); +} + +function replaceCondition(filters: readonly Filter[], target: Filter, next: Filter): Filter[] { + return normalizeFilters(filters.map((f) => (f === target ? next : f))); +} + +/** + * Replace a condition's operator — the field's only condition, or the one of + * `polarity` when a multi-valued field holds both. A no-op for an operator the + * field does not offer. + * + * Moving a tag condition onto the OTHER slot (an include switched to "includes + * none", or the reverse) merges it into the condition already there, and the + * operator just picked wins: the pill the reader changed is the one that says + * what they meant. + */ export function setFilterOperator( filters: readonly Filter[], field: FilterField, operator: FilterOperator, + polarity?: FilterPolarity, ): Filter[] { - if (!(requireField(field).operators as readonly string[]).includes(operator)) { + const target = findFilter(filters, field, polarity); + if (!target || !(requireField(field).operators as readonly string[]).includes(operator)) { return normalizeFilters(filters as unknown[]); } - return normalizeFilters(filters.map((f) => (f.field === field ? { ...f, operator } : f))); + // First in the list, so `normalizeFilters`' "first operator wins" merge keeps it. + return normalizeFilters([{ ...target, operator }, ...filters.filter((f) => f !== target)]); +} + +/** Remove a field's conditions — or only the one of `polarity`. */ +export function removeFilter( + filters: readonly Filter[], + field: FilterField, + polarity?: FilterPolarity, +): Filter[] { + return normalizeFilters( + filters.filter((f) => f.field !== field || (polarity !== undefined && filterPolarity(f) !== polarity)), + ); } -export function removeFilter(filters: readonly Filter[], field: FilterField): Filter[] { - return normalizeFilters(filters.filter((f) => f.field !== field)); +/** + * Whether the bar holds a condition only the Explorer can run: a tag include + * AND a tag exclude at once. A retention policy carries one `tags` list with + * one `tags_mode`, so it has no way to say both — handing such a bar to a + * policy would silently drop half of it and archive more than the reader saw. + */ +export function hasExplorerOnlyFilters(filters: readonly Filter[]): boolean { + return FILTER_FIELDS.some( + (d) => d.multiValued && findFilter(filters, d.field, 'include') && findFilter(filters, d.field, 'exclude'), + ); +} + +/** + * The bar minus what a policy cannot express — a multi-valued field's exclude + * slot when its include slot is also set. For a prefill that arrived anyway + * (a hand-edited link): the builder then SHOWS only the conditions the saved + * policy will match, rather than a pill it would quietly ignore. + */ +export function policyExpressibleFilters(filters: readonly Filter[]): Filter[] { + return normalizeFilters( + filters.filter( + (f) => + !(requireField(f.field).multiValued && + filterPolarity(f) === 'exclude' && + findFilter(filters, f.field, 'include')), + ), + ); } // ── Rendering ──────────────────────────────────────────────────────────────── @@ -726,6 +928,22 @@ function scalarModeFor(operator: FilterOperator): ScalarFilterMode { return operator === 'nin' ? 'nin' : 'in'; } +/** + * Whether a tag condition travels as `exclude_tags` rather than as `tags`. + * + * Only the exclude slot, and only beside an include slot: `tags` has one mode, + * so it can carry one of the two. A LONE exclusion keeps its historical + * encoding (`tags_mode=none`) — identical rows, and the shape retention + * policies and existing links already speak. + */ +function travelsAsExcludeTags(filter: Filter, filters: readonly Filter[]): boolean { + return ( + filter.field === 'label' && + filterPolarity(filter) === 'exclude' && + findFilter(filters, 'label', 'include') !== undefined + ); +} + /** * Translate the bar into `GET /memories` query params. * @@ -743,11 +961,16 @@ export function filtersToQueryParams( filters: readonly Filter[], ): Partial { const params: Partial = {}; + const normalized = normalizeFilters(filters as unknown[]); - for (const filter of normalizeFilters(filters as unknown[])) { + for (const filter of normalized) { const joined = filter.values.join(','); switch (filter.field) { case 'label': + if (travelsAsExcludeTags(filter, normalized)) { + params.exclude_tags = joined; + break; + } params.tags = joined; params.tags_mode = tagsModeFor(filter.operator); break; @@ -857,11 +1080,16 @@ export function filtersToFacetParams(filters: readonly Filter[]): Partial { const body: Partial = {}; + const normalized = normalizeFilters(filters as unknown[]); - for (const filter of normalizeFilters(filters as unknown[])) { + for (const filter of normalized) { const values = [...filter.values]; switch (filter.field) { case 'label': + if (travelsAsExcludeTags(filter, normalized)) { + body.exclude_tags = values; + break; + } body.tags = values; body.tags_mode = tagsModeFor(filter.operator); break; From 68bf0b892894b8014f87d3805d4b803abd2ed885 Mon Sep 17 00:00:00 2001 From: "mads.thines" Date: Tue, 20 Oct 2026 17:52:40 +0000 Subject: [PATCH 5/9] feat(web): filter in / filter out icon pair on detail-panel values FilterableMetaValue's flyout is now two icon buttons (magnifier plus / minus), each explained by its own tooltip; a value already filtered in or out shows that button pressed and the value itself tinted or struck through. Enter / Shift+Enter on the value do the same from the keyboard. The filter menu marks excluded values with a minus instead of a check. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> --- .../web/src/components/lore/FilterMenu.tsx | 32 ++- .../src/components/lore/LessonDetailSheet.tsx | 94 ++++---- .../src/components/memory/MemoryOrigin.tsx | 33 +-- .../src/components/ui/FilterableMetaValue.tsx | 218 +++++++++++++----- packages/web/src/components/ui/TagsField.tsx | 47 ++-- .../web/src/lib/filter-from-metadata.spec.ts | 105 +++++++-- packages/web/src/lib/filter-from-metadata.ts | 101 +++++--- 7 files changed, 435 insertions(+), 195 deletions(-) diff --git a/packages/web/src/components/lore/FilterMenu.tsx b/packages/web/src/components/lore/FilterMenu.tsx index c08614d9a..14d29d510 100644 --- a/packages/web/src/components/lore/FilterMenu.tsx +++ b/packages/web/src/components/lore/FilterMenu.tsx @@ -134,6 +134,7 @@ import { Boxes, CalendarPlus, Check, + Minus, ChevronLeft, ChevronRight, Download, @@ -165,6 +166,7 @@ import { rootSuggestions, searchOptions, selectedValues, + valuePolarity, type FacetValue, type Filter, type FilterField, @@ -879,7 +881,13 @@ export function FilterMenu({ ) : field ? ( valueRows.map((option, i) => { - const isSelected = selectedValues(filters, field).includes(option.value); + // Excluded values (a tag in "includes none", a host in "is not") are + // selected too — unchecking takes the exclusion off — but marked with + // a minus rather than a check, so an exclusion never reads as an + // inclusion when a tag has both pills. + const polarity = valuePolarity(filters, field, option.value); + const isSelected = polarity !== null; + const isExcluded = polarity === 'exclude'; return ( + activate('include')} + /> + {onFilterOut && ( + <> + + activate('exclude')} + /> + + )} , document.body, )} ); } + +/** Hover colour, and the pressed look, per polarity: accent for in, error red for out. */ +const ACTION_TONE: Record = { + include: { + hover: 'hover:text-[var(--color-accent)]', + pressed: 'bg-[var(--color-accent-subtle)] text-[var(--color-accent)]', + }, + exclude: { + hover: 'hover:text-[var(--color-error)]', + pressed: 'bg-[var(--color-error)]/15 text-[var(--color-error)]', + }, +}; + +function FilterActionButton({ + polarity, + subject, + state, + icon: Icon, + onActivate, +}: { + polarity: FilterPolarity; + subject: string; + state: FilterPolarity | null; + icon: LucideIcon; + onActivate: () => void; +}) { + const explanation = filterActionLabel(polarity, subject, state); + const pressed = state === polarity; + const tone = ACTION_TONE[polarity]; + return ( + + + + ); +} diff --git a/packages/web/src/components/ui/TagsField.tsx b/packages/web/src/components/ui/TagsField.tsx index cc1c0e2fe..249b6dd9d 100644 --- a/packages/web/src/components/ui/TagsField.tsx +++ b/packages/web/src/components/ui/TagsField.tsx @@ -40,6 +40,7 @@ import { useState, useRef, useCallback, useId } from 'react'; import { AnimatePresence, motion } from 'motion/react'; import { X, Tag } from 'lucide-react'; +import type { FilterPolarity } from '@/lib/filters'; import { FilterableMetaValue } from './FilterableMetaValue'; export interface TagsFieldProps { @@ -59,20 +60,22 @@ export interface TagsFieldProps { className?: string; /** * R3 (lore-explorer-panel-nav-facets): when provided, every chip gets a - * hover-reveal "Filter by this label" affordance that calls back with the - * bare tag value. The sole caller today is `LessonDetailSheet` — an editing - * context (e.g. a future bulk-tag editor) would simply omit this. + * hover-reveal "filter in / filter out" affordance that calls back with the + * bare tag value and which of the two was pressed. The sole caller today is + * `LessonDetailSheet` — an editing context (e.g. a future bulk-tag editor) + * would simply omit this. */ - onTagFilter?: (tag: string) => void; + onTagFilter?: (tag: string, polarity: FilterPolarity) => void; /** - * Whether a given tag is currently part of the applied Label filter - * (`isMetaValueActiveFilter`, `lib/filter-from-metadata.ts`). When it - * returns true the chip gets the same subtle accent-orange "applied" tint - * as a committed `FilterPill` — a passive visual state, independent of the - * `onTagFilter` hover affordance above. Omitted (or absent) renders every - * chip in its neutral style, matching the pre-existing look. + * Whether a given tag is currently filtered in or out by the applied tag + * filter (`metaValueFilterState`, `lib/filter-from-metadata.ts`), or neither. + * "In" gives the chip the same subtle accent-orange "applied" tint as a + * committed `FilterPill`; "out" strikes it through and mutes it — passive + * visual states, independent of the `onTagFilter` hover affordance above. + * Omitted (or absent) renders every chip in its neutral style, matching the + * pre-existing look. */ - isTagActive?: (tag: string) => boolean; + tagFilterState?: (tag: string) => FilterPolarity | null; } export function TagsField({ @@ -84,7 +87,7 @@ export function TagsField({ maxTags = 20, className = '', onTagFilter, - isTagActive, + tagFilterState, }: TagsFieldProps) { const [inputValue, setInputValue] = useState(''); const inputRef = useRef(null); @@ -141,7 +144,7 @@ export function TagsField({ > {tags.map((tag, i) => { - const active = isTagActive?.(tag) ?? false; + const filterState = tagFilterState?.(tag) ?? null; return ( {onTagFilter ? ( - onTagFilter(tag)}> + onTagFilter(tag, 'include')} + onFilterOut={() => onTagFilter(tag, 'exclude')} + state={filterState} + cueOnValue={false} + > {tag} ) : ( diff --git a/packages/web/src/lib/filter-from-metadata.spec.ts b/packages/web/src/lib/filter-from-metadata.spec.ts index 672a677da..8632a6b39 100644 --- a/packages/web/src/lib/filter-from-metadata.spec.ts +++ b/packages/web/src/lib/filter-from-metadata.spec.ts @@ -1,7 +1,9 @@ import { describe, it, expect } from 'vitest'; import { applyMetadataFilterHref, - isMetaValueActiveFilter, + filterActionLabel, + filterAnchorLabel, + metaValueFilterState, metadataFilterTargets, type CurrentExplorerParams, type MetadataFilterTarget, @@ -185,7 +187,59 @@ describe('applyMetadataFilterHref', () => { }); }); -describe('isMetaValueActiveFilter', () => { +describe('applyMetadataFilterHref — filter out', () => { + const tagTarget = (value: string): MetadataFilterTarget => ({ + kind: 'filter', + field: 'label', + value, + label: `Filter by label "${value}"`, + }); + const filtersOf = (href: string) => + JSON.parse(new URLSearchParams(href.split('?')[1]).get('filters') ?? 'null'); + + it('writes a NOT condition for the value', () => { + expect(filtersOf(applyMetadataFilterHref(baseParams, tagTarget('flaky'), 'exclude'))).toEqual([ + { field: 'label', operator: 'nin', values: ['flaky'] }, + ]); + }); + + it('ANDs the exclusion with a tag already filtered in', () => { + const current: CurrentExplorerParams = { + ...baseParams, + filters: [{ field: 'label', operator: 'all', values: ['perf'] }], + }; + expect(filtersOf(applyMetadataFilterHref(current, tagTarget('flaky'), 'exclude'))).toEqual([ + { field: 'label', operator: 'all', values: ['perf'] }, + { field: 'label', operator: 'nin', values: ['flaky'] }, + ]); + }); + + it('filter in still defaults when no polarity is given', () => { + expect(filtersOf(applyMetadataFilterHref(baseParams, tagTarget('perf')))).toEqual([ + { field: 'label', operator: 'all', values: ['perf'] }, + ]); + }); +}); + +describe('filterActionLabel / filterAnchorLabel', () => { + it('explains each button, and how to undo a pressed one', () => { + expect(filterActionLabel('include', 'tag "perf"', null)).toBe('Filter in — show only memories with tag "perf"'); + expect(filterActionLabel('exclude', 'tag "perf"', null)).toBe('Filter out — hide memories with tag "perf"'); + expect(filterActionLabel('include', 'tag "perf"', 'include')).toBe( + 'Filtered in — click to remove tag "perf" from the filter', + ); + expect(filterActionLabel('exclude', 'tag "perf"', 'exclude')).toBe('Filtered out — click to stop hiding tag "perf"'); + // The OTHER polarity being active does not make this button pressed. + expect(filterActionLabel('include', 'tag "perf"', 'exclude')).toBe('Filter in — show only memories with tag "perf"'); + }); + + it('names the keyboard path on the value itself', () => { + expect(filterAnchorLabel('host "aw"', true)).toBe('host "aw" — press Enter to filter in, Shift+Enter to filter out'); + expect(filterAnchorLabel('scope "global"', false)).toBe('scope "global" — press Enter to filter in'); + }); +}); + +describe('metaValueFilterState', () => { const labelTarget: MetadataFilterTarget = { kind: 'filter', field: 'label', @@ -195,19 +249,30 @@ describe('isMetaValueActiveFilter', () => { it('matches a label target whose value is selected under `all` (AC-25)', () => { const filters: Filter[] = [{ field: 'label', operator: 'all', values: ['agent0::incident-memory'] }]; - expect(isMetaValueActiveFilter(labelTarget, filters, null)).toBe(true); + expect(metaValueFilterState(labelTarget, filters, null)).toBe('include'); }); it('does not match a label target whose value is not selected', () => { const filters: Filter[] = [{ field: 'label', operator: 'all', values: ['dataset::default'] }]; - expect(isMetaValueActiveFilter(labelTarget, filters, null)).toBe(false); + expect(metaValueFilterState(labelTarget, filters, null)).toBeNull(); }); - it('matches regardless of operator — `in`, `nin`, and `all` all count as applied', () => { - for (const operator of ['in', 'nin', 'all'] as const) { + it('reads `in` and `all` as filtered in, and `nin` as filtered out', () => { + for (const operator of ['in', 'all'] as const) { const filters: Filter[] = [{ field: 'label', operator, values: ['agent0::incident-memory'] }]; - expect(isMetaValueActiveFilter(labelTarget, filters, null)).toBe(true); + expect(metaValueFilterState(labelTarget, filters, null)).toBe('include'); } + const excluded: Filter[] = [{ field: 'label', operator: 'nin', values: ['agent0::incident-memory'] }]; + expect(metaValueFilterState(labelTarget, excluded, null)).toBe('exclude'); + }); + + it('reads each tag slot separately when a tag is included and another excluded', () => { + const filters: Filter[] = [ + { field: 'label', operator: 'all', values: ['perf'] }, + { field: 'label', operator: 'nin', values: ['agent0::incident-memory'] }, + ]; + expect(metaValueFilterState(labelTarget, filters, null)).toBe('exclude'); + expect(metaValueFilterState({ ...labelTarget, value: 'perf' }, filters, null)).toBe('include'); }); it('matches a scope target iff it equals the current ?scope= exactly', () => { @@ -216,9 +281,9 @@ describe('isMetaValueActiveFilter', () => { scope: 'repo::mthines/lorekit', label: 'Filter by scope repo::mthines/lorekit', }; - expect(isMetaValueActiveFilter(target, [], 'repo::mthines/lorekit')).toBe(true); - expect(isMetaValueActiveFilter(target, [], 'repo::other/repo')).toBe(false); - expect(isMetaValueActiveFilter(target, [], null)).toBe(false); + expect(metaValueFilterState(target, [], 'repo::mthines/lorekit')).toBe('include'); + expect(metaValueFilterState(target, [], 'repo::other/repo')).toBeNull(); + expect(metaValueFilterState(target, [], null)).toBeNull(); }); it('matches repo/branch/pr filter targets the same way as label', () => { @@ -245,10 +310,10 @@ describe('isMetaValueActiveFilter', () => { { field: 'branch', operator: 'in', values: ['main'] }, { field: 'pr', operator: 'in', values: ['482'] }, ]; - expect(isMetaValueActiveFilter(repoTarget, filters, null)).toBe(true); - expect(isMetaValueActiveFilter(branchTarget, filters, null)).toBe(true); - expect(isMetaValueActiveFilter(prTarget, filters, null)).toBe(true); - expect(isMetaValueActiveFilter({ ...prTarget, value: '999' }, filters, null)).toBe(false); + expect(metaValueFilterState(repoTarget, filters, null)).toBe('include'); + expect(metaValueFilterState(branchTarget, filters, null)).toBe('include'); + expect(metaValueFilterState(prTarget, filters, null)).toBe('include'); + expect(metaValueFilterState({ ...prTarget, value: '999' }, filters, null)).toBeNull(); }); it('matches kind/host/agent/trigger filter targets the same way as label', () => { @@ -264,14 +329,14 @@ describe('isMetaValueActiveFilter', () => { value, label: `Filter by ${field} "${value}"`, }); - expect(isMetaValueActiveFilter(target('kind', 'lesson'), filters, null)).toBe(true); - expect(isMetaValueActiveFilter(target('host', 'reviewer'), filters, null)).toBe(true); - expect(isMetaValueActiveFilter(target('agent', 'aw'), filters, null)).toBe(true); - expect(isMetaValueActiveFilter(target('trigger', 'stuck-loop'), filters, null)).toBe(true); - expect(isMetaValueActiveFilter(target('kind', 'bus'), filters, null)).toBe(false); + expect(metaValueFilterState(target('kind', 'lesson'), filters, null)).toBe('include'); + expect(metaValueFilterState(target('host', 'reviewer'), filters, null)).toBe('include'); + expect(metaValueFilterState(target('agent', 'aw'), filters, null)).toBe('include'); + expect(metaValueFilterState(target('trigger', 'stuck-loop'), filters, null)).toBe('include'); + expect(metaValueFilterState(target('kind', 'bus'), filters, null)).toBeNull(); }); it('returns false when no filter is applied at all', () => { - expect(isMetaValueActiveFilter(labelTarget, [], null)).toBe(false); + expect(metaValueFilterState(labelTarget, [], null)).toBeNull(); }); }); diff --git a/packages/web/src/lib/filter-from-metadata.ts b/packages/web/src/lib/filter-from-metadata.ts index da2575638..ca87e81ac 100644 --- a/packages/web/src/lib/filter-from-metadata.ts +++ b/packages/web/src/lib/filter-from-metadata.ts @@ -11,19 +11,20 @@ * the component. * * Deliberately delegates to `filters.ts` rather than re-deriving any of its - * logic (code-quality: one source of truth) — `toggleFilterValue`, + * logic (code-quality: one source of truth) — `applyValueFilter`, * `resolveFilters`, and `filtersParamValue` are the SAME functions the - * Explorer's own filter bar uses, so a value toggled from the panel produces - * the exact `?filters=` shape the bar would have written itself. + * Explorer's own filter bar uses, so a value filtered in or out from the panel + * produces the exact `?filters=` shape the bar would have written itself. */ import { + applyValueFilter, filtersParamValue, - isValueSelected, requireField, - toggleFilterValue, + valuePolarity, type Filter, type FilterField, + type FilterPolarity, } from './filters'; import type { LessonEntry } from '@/components/lore/LessonCard'; @@ -177,16 +178,19 @@ export interface CurrentExplorerParams { /** * The next `/lore?…` href for applying `target` from the panel. * - * A `filter` target toggles the value into the SAME `Filter[]` shape the - * Explorer's filter bar writes (`toggleFilterValue` + `filtersParamValue` — - * no new URL param, AC-6); a `scope` target replaces `?scope=` instead - * (AC-7). Either way `?lesson=` is preserved when present, so the panel - * stays open on the SAME memory after the navigation (Decision D3/AC-8) even - * if that memory no longer matches the newly-narrowed list. + * A `filter` target filters the value in or out (`polarity`) in the SAME + * `Filter[]` shape the Explorer's filter bar writes (`applyValueFilter` + + * `filtersParamValue` — no new URL param, AC-6); a `scope` target replaces + * `?scope=` instead (AC-7) and has no "out": the API has no "every scope but + * this one" filter, so the panel never offers it. Either way `?lesson=` is + * preserved when present, so the panel stays open on the SAME memory after the + * navigation (Decision D3/AC-8) even if that memory no longer matches the + * newly-narrowed list. */ export function applyMetadataFilterHref( current: CurrentExplorerParams, target: MetadataFilterTarget, + polarity: FilterPolarity = 'include', ): string { const params = new URLSearchParams(); @@ -196,7 +200,7 @@ export function applyMetadataFilterHref( const filtersValue = filtersParamValue(current.filters, current.legacyTags, current.legacyOwner); if (filtersValue !== null) params.set('filters', JSON.stringify(filtersValue)); } else { - const nextFilters = toggleFilterValue(current.filters, target.field, target.value); + const nextFilters = applyValueFilter(current.filters, target.field, target.value, polarity); const filtersValue = filtersParamValue(nextFilters, current.legacyTags, current.legacyOwner); if (filtersValue !== null) params.set('filters', JSON.stringify(filtersValue)); if (current.scope !== null) params.set('scope', JSON.stringify(current.scope)); @@ -215,32 +219,61 @@ export function applyMetadataFilterHref( // ── Active-filter matching (R9 — "which of this memory's values am I already filtering by") ── /** - * Whether `target` matches the currently-applied filter/scope state — i.e. - * whether ACTIVATING it would be a no-op because it is already applied. Used - * to render the metadata value it came from (a tag chip, the scope badge, a - * kind/host/agent/trigger value, an origin repo/branch/PR chip) with the same - * "this is active" accent the filter bar's own committed pills use, so the - * panel answers "which of THIS memory's attributes are the ones I'm filtering - * by" without a parallel matching scheme. + * Whether `target`'s value is currently filtered IN, filtered OUT, or neither + * (`null`). Drives two things: the value's own cue in the panel (the accent + * tint a committed `FilterPill` uses for "in", a struck-through muted style for + * "out"), and which of the panel's two buttons reads as pressed — pressing it + * again is how the reader takes that filter back off. * - * A `filter` target matches when its value is currently selected for its - * field — reusing {@link isValueSelected}, the SAME membership check - * `FilterMenu`'s value checkboxes use, so a value that reads "selected" in - * the menu reads "active" here too. Deliberately operator-agnostic: whether - * the field is combined with `in` (is/is either of), `nin` (is not), or - * `all` (includes all) does not change whether THIS value is part of the - * set the field currently names — the panel is answering "is this among - * what's applied", not restating the qualifier a `FilterPill` already shows. + * A `filter` target reads its polarity from {@link valuePolarity}: the slot + * that names the value, so a tag in "includes none" reads as out and a host in + * "is either of" reads as in. A value no condition names is `null`, even when + * a condition happens to rule it out — nothing on screen says so. * - * A `scope` target matches when it equals the current `?scope=` EXACTLY — - * scope is a single value, not a set, so there is no operator to consider, - * and `null` (no scope filter active) never matches any scope string. + * A `scope` target is `include` when it equals the current `?scope=` EXACTLY — + * scope is a single value with no "out", and `null` (no scope filter active) + * never matches any scope string. */ -export function isMetaValueActiveFilter( +export function metaValueFilterState( target: MetadataFilterTarget, filters: readonly Filter[], scope: string | null, -): boolean { - if (target.kind === 'scope') return scope !== null && scope === target.scope; - return isValueSelected(filters, target.field, target.value); +): FilterPolarity | null { + if (target.kind === 'scope') return scope !== null && scope === target.scope ? 'include' : null; + return valuePolarity(filters, target.field, target.value); +} + +// ── Copy for the panel's two buttons ───────────────────────────────────────── + +/** + * The explanation shown in each button's tooltip (and used as its accessible + * name). `subject` names the value in words — `tag "perf"`, `host "reviewer"`. + * A pressed button says how to take the filter back off, since pressing it + * again is exactly that. + */ +export function filterActionLabel( + polarity: FilterPolarity, + subject: string, + state: FilterPolarity | null, +): string { + const pressed = state === polarity; + if (polarity === 'include') { + return pressed + ? `Filtered in — click to remove ${subject} from the filter` + : `Filter in — show only memories with ${subject}`; + } + return pressed + ? `Filtered out — click to stop hiding ${subject}` + : `Filter out — hide memories with ${subject}`; +} + +/** + * The hovered value's own accessible name: what it is, and the two keys that + * act on it without the mouse (the flyout's buttons are pointer-only, because a + * portaled panel at the end of the DOM is an out-of-order Tab stop). + */ +export function filterAnchorLabel(subject: string, canFilterOut: boolean): string { + return canFilterOut + ? `${subject} — press Enter to filter in, Shift+Enter to filter out` + : `${subject} — press Enter to filter in`; } From 2285d6a1223f6fceee3e805a405ab5e57c05d6bd Mon Sep 17 00:00:00 2001 From: "mads.thines" Date: Tue, 20 Oct 2026 17:53:23 +0000 Subject: [PATCH 6/9] docs: document exclude_tags and filter in / filter out; regenerate llms.txt Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> --- docs/decisions.md | 2 ++ docs/key-files.md | 1 + packages/schemas/src/llms/template.md | 5 +++++ packages/web/public/llms.txt | 5 +++++ packages/web/src/content/docs/tags.mdx | 4 ++++ 5 files changed, 17 insertions(+) diff --git a/docs/decisions.md b/docs/decisions.md index 505f7c00f..ee78a276b 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -20,6 +20,8 @@ rationale before proposing a change that reverses one. **The Lore Explorer filters through ONE two-level command menu and a row of pills — never one picker per dimension, and never a client-side narrowing.** The Explorer filtered on labels alone, behind a bespoke popover (`LabelFilter`). It now filters on nine dimensions — label, kind, host, owner, agent, trigger, repo, branch, pull request — and the shape that scales is Linear's: a single **Filter** trigger opening a menu whose first level is the DIMENSION list and whose second is that dimension's values, with committed conditions rendered below as three-segment pills. A picker per dimension would put nine controls in a row that has to survive a phone and would still not answer "what else can I filter by?"; here a new dimension costs one row in `FILTER_FIELDS` and nothing else. **Semantics are OR within a dimension, AND across dimensions**, because that is the only combination a flat pill row can render without inventing a precedence grammar — cross-dimension OR and nested groups stay in `POST /memories/search`'s `filter` tree, which already expresses them. **Every dimension is filtered SERVER-side** (`GET /memories` gained `source_agent` / `trigger` / `origin_repo` / `origin_branch` / `origin_pr` plus a `*_mode` of `in`/`nin`, `tags_mode` gained `none`, and `owner` — value `personal` or an org slug — landed in 00064), never by narrowing the loaded pages: those are keyset pages, so a client-side narrowing of a filter the user can actually type is "no matches" on page one and matches on page three. **Owner was the last exception** — a separate client-side `OwnershipFilterBar` over the loaded pages — and 00064 folded it into the same machinery as a real facet dimension: `lorekit_memory_facets`, `lorekit_memory_activity` and `GET /memories` all resolve `personal` → `org_id is null` and an org slug → the caller's member org (slugs resolve against member orgs only, which the visibility predicate already guarantees), so the owner filter agrees with the list, the drill-down counts and the stat header like every other dimension. The `?owner=` deep link the accept-invite flow used is superseded: it is still READ (a `'personal'` value → an `owner` filter; `'all'` / the legacy `{orgId}` form → none) but never written, and the flow now deep-links a slug-keyed `owner` filter directly. **The value catalog is its own query, never derived from the loaded pages** (`GET /memories/facets`, 00052) for `useTagCatalog`'s reason, which only sharpens with more dimensions — derived from the loaded pages the options would shrink to whatever the current page returned, and the cross-dimension type-ahead could only ever surface values already visible in the list, which is exactly when you did not need it. The ENDPOINT is no longer filter-INDEPENDENT: **00057 made the counts drill-down** — it takes the caller's active filters and counts each dimension with every OTHER one applied but not its own (self-exclusion), so a value's count is what selecting it would yield while the dimension you are standing in still shows everything you could switch to. Unfiltered, it returns 00052's global catalog unchanged; the Explorer now passes its active filter bar and the selected scope (`listFacetsPostRequest` ← `filtersToFacetBody` + `useFacetCatalog`'s `scope`, keyed on both — `listFacetsRequest` ← `filtersToFacetParams` is the equivalent GET form, kept and spec-covered but no longer called from this package), so its menu counts drill down as you build the bar and match the scoped list. `kind`/`host`/`owner` have `FILTER_FIELDS` rows now, so all nine dimensions are filterable — `FacetName` and `FilterField` are one-to-one, and `filters.spec.ts` executes that against `MemoryFacetSchema` so a facet added server-side without a pill fails a test instead of being silently dropped. The one residual widen/switch loss is that a value counting zero under the other filters emits no row, so it drops out of the menu until the filter is cleared. The counts do NOT mirror `q` / `key` — with a search active they are upper bounds, not exact yields, because mirroring `q` would mean a second implementation of `likeNeedle`'s escaping in plpgsql. **`created_since` / `created_until` and the five retention thresholds ARE mirrored as of 00108**, and the gap they closed was a disagreement rather than a missing feature: only the list applied the thresholds, so `max_opened_count => 0` narrowed the rows and left the facet counts, the stat cards and the matrix counting the un-narrowed population — every number on the page describing a different set from the one below it. All four readers now compose ONE shared `lorekit_match_retention`, so a count cannot drift from the list it describes; it is applied in the `base` CTE's WHERE rather than as an `ok_*` flag, which is what makes a threshold narrow even the SELF-EXCLUDED dimension — deliberately unlike a dimension, because 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. **Selection is URL-backed** (`?filters=`), so a filtered view is a link; the pre-existing `?tags=` param is still READ (never written) and translated by `filtersFromLegacyTags` into a `label` filter with the `all` operator, which is what it has always meant — links in PRs, Slack and `lorekit link` output keep resolving to the filter they name. The decision logic is pure and node-tested in `lib/filters.ts`; `FilterMenu`/`FilterPill` are the shell. **Do not re-add a per-dimension trigger, a client-side dimension filter, or a second catalog query.** **The Explorer reads those server-side filters over a JSON BODY** (`POST /memories/list` / `/facets` / `/activity`), not a query string: a query string caps each dimension at 2048 characters (`ValueListSchema`), so the number of values a dimension can hold depended on how long the values happened to be — with realistic ~28-character host names the Explorer stopped loading somewhere between 50 and 75 selected values and rendered only "Failed to load memories. Please refresh." — and even under that cap eight dimensions compose a URL past what the gateway accepts, which fails with no LoreKit error envelope at all. Raising the cap only moves the first wall and makes the second arrive first, so this is a transport change rather than a bigger number. **The GET forms stay fully supported and unchanged** (the CLI, MCP and API-token callers use them, and a link carrying a few filters is better as a URL); the two transports cannot drift because neither handler decides anything — `dimensionsFromQuery` and `dimensionsFromBody` (`@lorekit/schemas/dimensions`) both produce ONE `MemoryDimensions` and a single predicate function turns that into SQL, so a dimension is one field and one line rather than two of each. Body dimensions are arrays bounded at 1000 values × 512 characters — an explicit limit rather than an emergent one — and both forms report under the SAME `usage_events.tool_name`, because splitting `memory.list` in two would have read as an outage on the day the dashboard switched. +**Tags are the one dimension with TWO pills: an include and an exclude (00114).** A row carries many labels, so "includes perf" and "includes none of flaky" are independent conditions that can both hold — and the old one-pill model could not say both, because `tags` has one mode. The bar now keeps an include slot (`all`/`any`) and an exclude slot (`nin`) for `label` (`FilterFieldDescriptor.multiValued`), ANDed together; on the wire the exclude slot travels as `exclude_tags` only beside an include, and as the historical `tags_mode=none` on its own. Every SINGLE-valued dimension keeps one pill, because "is A or B" AND "is not X" simplifies exactly into one condition — `applyValueFilter` (the detail panel's filter in / filter out) does that simplification, and moves a value between polarities rather than ever producing "required AND forbidden". A retention policy carries one `tags` condition, so the Explorer withholds "Create retention policy" while a tag is both included and excluded (`hasExplorerOnlyFilters`), mirroring how it already withholds it for the reverse activity directions. + **00110 refines self-exclusion for the one dimension where it was misleading: labels under AND mode.** Self-exclusion (00057) always meant "count every OTHER filter applied, but not this dimension's own" — right for a scalar dimension (kind, host, agent, trigger, repo, branch, pr, owner: there is only ever one selected value to exclude) and right for labels under `tags_mode='any'` (any ONE of several OR'd values could be swapped for another). It stopped being right for labels under `tags_mode='all'`: with `perf` already selected in AND mode, self-exclusion reported a candidate label's TOTAL count across every row carrying it, including rows that never carried `perf` at all — so picking a second label could show a count larger than the number of rows the resulting AND-of-both-labels filter would actually match, which reads as a promise the click does not keep. `lorekit_memory_facets` (migration 00110, same signature — `create or replace`, no drop) fixes this by making the label cell's `matched` flag additionally require every already-selected tag (`ok_tag`) whenever `p_tags_mode = 'all'`; under `tags_mode='any'` the added condition is a no-op and self-exclusion is bit-for-bit unchanged, and no scalar dimension's `matched` expression changed at all. The fix is WITHIN one dimension, never across dimensions — a `kind` filter still narrows the `label` counts the same way it always did (cross-group AND is untouched); only the label facet's relationship to OTHER already-selected labels changed. The migration also switches every facet's counting from "filter matching rows, then group" to "enumerate every candidate value from the full (retention/scope/tenant-filtered) population, then `count(*) filter (where matched)`" — so a value with zero matches under the current filters now reports `count: 0` and stays in the menu (the reader can see there is nothing there before clicking) rather than silently disappearing, which is what "filter then group" always did to a value whose every row got excluded. `supabase/tests/migrations.test.sql`'s own added block asserts all four: co-occurrence under AND, the zero-stays row, unchanged self-exclusion under OR, and that a cross-group (non-label) filter still narrows the label counts. ## Mobile transient selection surfaces use the BottomSheet primitive diff --git a/docs/key-files.md b/docs/key-files.md index 5fb603597..9ff2c8d17 100644 --- a/docs/key-files.md +++ b/docs/key-files.md @@ -181,6 +181,7 @@ else. Grouped loosely by subsystem in migration/PR order. | `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 | +| `supabase/migrations/00114_exclude_tags_dimension.sql` | `p_exclude_tags text[]` appended to `lorekit_memory_list`/`_facets`/`_activity`/`_pivot` (each dropped at its exact 00113 signature first): an always-applied "carries none of these labels" conjunct, independent of `p_tags_mode`, reusing `lorekit_match_tags(…, 'none')` — so "tagged a AND NOT tagged b" is expressible. In `_facets` it is an `ok_tag_exclusion` flag the tag facet NEVER self-excludes (an excluded label stays listed at `count = 0`); `_pivot` narrows `base`; the list binds it as `$45`. The dashboard sends it only when a tag include is also active — a lone exclusion still travels as `tags_mode=none`, and retention policies never gain it. Pinned by §114 | | `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 | diff --git a/packages/schemas/src/llms/template.md b/packages/schemas/src/llms/template.md index 06bfac120..e52736036 100644 --- a/packages/schemas/src/llms/template.md +++ b/packages/schemas/src/llms/template.md @@ -214,6 +214,11 @@ source_agent=aw&origin_branch=main&tags=flaky&tags_mode=none" Each dimension takes a comma-separated value list plus an optional `_mode` of `in` (default) or `nin` (negate); labels use `tags_mode` of `any` (default), `all`, or `none`. +`exclude_tags` (comma-separated) is a separate "carries none of these labels" condition ANDed with +`tags` in any mode, so `tags=perf&tags_mode=all&exclude_tags=flaky` is "tagged perf, not tagged +flaky". `GET /memories/facets`, `/activity` and `/pivot` (and their `POST` body forms) take it too. +In the Explorer, tags can carry an include pill and an exclude pill at once, and a memory's detail +panel offers filter in / filter out on every metadata value. `kind` and `host` are the memory TAXONOMY: `kind` is the bucket type — a closed vocabulary of `lesson`, `bus` and `signal` — and `host` is the skill or agent that owns the bucket. Together they diff --git a/packages/web/public/llms.txt b/packages/web/public/llms.txt index 018ca82b7..4b116bc49 100644 --- a/packages/web/public/llms.txt +++ b/packages/web/public/llms.txt @@ -742,6 +742,11 @@ source_agent=aw&origin_branch=main&tags=flaky&tags_mode=none" Each dimension takes a comma-separated value list plus an optional `_mode` of `in` (default) or `nin` (negate); labels use `tags_mode` of `any` (default), `all`, or `none`. +`exclude_tags` (comma-separated) is a separate "carries none of these labels" condition ANDed with +`tags` in any mode, so `tags=perf&tags_mode=all&exclude_tags=flaky` is "tagged perf, not tagged +flaky". `GET /memories/facets`, `/activity` and `/pivot` (and their `POST` body forms) take it too. +In the Explorer, tags can carry an include pill and an exclude pill at once, and a memory's detail +panel offers filter in / filter out on every metadata value. `kind` and `host` are the memory TAXONOMY: `kind` is the bucket type — a closed vocabulary of `lesson`, `bus` and `signal` — and `host` is the skill or agent that owns the bucket. Together they diff --git a/packages/web/src/content/docs/tags.mdx b/packages/web/src/content/docs/tags.mdx index 2c9164203..5f2e0a29d 100644 --- a/packages/web/src/content/docs/tags.mdx +++ b/packages/web/src/content/docs/tags.mdx @@ -97,6 +97,10 @@ The [Explorer](/lore) filters on nine dimensions, not just tags. Press the **Fil Values inside one dimension combine with **OR**; dimensions combine with **AND**. So "Agent is either of `aw`, `claude`" **and** "Branch is `main`" is one filter set, and every filter is in the URL — copy the address to share the exact view. +Tags are the one dimension that can hold **two pills at once** — one that includes and one that excludes — because a memory carries many tags. "Tag includes all `perf`" **and** "Tag includes none `flaky`" shows memories tagged `perf` that are not tagged `flaky`. Every other dimension has a single value per memory, so its include and exclude collapse into one pill. + +You can also filter straight from a memory's detail panel. Hover any tag, kind, host, agent, trigger, repository, branch or pull request and two icons appear: **filter in** (magnifier +) shows only memories with that value, **filter out** (magnifier −) hides them. Hover an icon to see exactly what it will do. The filter is added to whatever is already applied — filtering out a tag while another tag is filtered in gives you both conditions — and a value you have already filtered in or out shows that icon pressed, so clicking it again takes the filter back off. From the keyboard, focus the value and press `Enter` to filter in or `Shift+Enter` to filter out. The scope offers filter in only. + Two shortcuts worth knowing: - **Type the value, not the dimension.** With the menu open, typing `main` surfaces `Branch → main` directly, so you never have to choose "Branch" first. From 537c8119cd097a2af6631c5668a17ae99f5e7c8c Mon Sep 17 00:00:00 2001 From: "mads.thines" Date: Tue, 20 Oct 2026 18:00:49 +0000 Subject: [PATCH 7/9] fix: regenerate BYOD bootstrap for 00114; keep a lone tag pill's key stable A pill keyed by its slot remounted when its operator flipped between include and exclude, leaving the exiting pill in the DOM. Key a field's first pill by the field; only a tag's second pill gets its own key. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> --- .../web/src/components/lore/FilterBar.tsx | 4 +- packages/web/src/lib/filters-polarity.spec.ts | 11 +- packages/web/src/lib/filters.ts | 12 +- supabase/byod/bootstrap.sql | 986 +++++++++++++++++- 4 files changed, 1003 insertions(+), 10 deletions(-) diff --git a/packages/web/src/components/lore/FilterBar.tsx b/packages/web/src/components/lore/FilterBar.tsx index 4bf8a1092..41c640b28 100644 --- a/packages/web/src/components/lore/FilterBar.tsx +++ b/packages/web/src/components/lore/FilterBar.tsx @@ -43,7 +43,7 @@ import { FilterMenu } from './FilterMenu'; import { FilterPill } from './FilterPill'; import { RetentionPill } from './RetentionPill'; import { - filterKey, + filterPillKey, filterPolarity, filtersPhrase, type FacetValue, @@ -207,7 +207,7 @@ export function FilterPillRow({ {filters.map((filter) => ( onOperatorChange(filter.field, op, filterPolarity(filter))} onEditValues={() => onEditField(filter.field)} diff --git a/packages/web/src/lib/filters-polarity.spec.ts b/packages/web/src/lib/filters-polarity.spec.ts index 76fce2279..8115ec183 100644 --- a/packages/web/src/lib/filters-polarity.spec.ts +++ b/packages/web/src/lib/filters-polarity.spec.ts @@ -11,7 +11,7 @@ import { describe, it, expect } from 'vitest'; import { ListMemoriesBodySchema, ListMemoriesQuerySchema } from '@lorekit/schemas/memory'; import { applyValueFilter, - filterKey, + filterPillKey, filtersToBody, filtersToQueryParams, findFilter, @@ -46,9 +46,12 @@ describe('normalizeFilters — the tag dimension keeps two slots', () => { expect(normalizeFilters([hostIs('a'), hostNot('b')])).toEqual([hostIs('a', 'b')]); }); - it('gives each pill its own key', () => { - expect(filterKey(tagIn('a'))).not.toBe(filterKey(tagOut('b'))); - expect(filterKey(hostIs('a'))).toBe(filterKey(hostNot('b'))); + it('keys a field\'s first pill by the field, so an operator change rewrites it in place', () => { + const include = tagIn('a'); + const exclude = tagOut('a'); + expect(filterPillKey(include, [include])).toBe(filterPillKey(exclude, [exclude])); + const second = tagOut('b'); + expect(filterPillKey(include, [include, second])).not.toBe(filterPillKey(second, [include, second])); }); }); diff --git a/packages/web/src/lib/filters.ts b/packages/web/src/lib/filters.ts index d6c0ff0ff..a110c8bb4 100644 --- a/packages/web/src/lib/filters.ts +++ b/packages/web/src/lib/filters.ts @@ -356,9 +356,15 @@ function slotKey(field: FilterField, operator: FilterOperator): string { return requireField(field).multiValued && operator === 'nin' ? `${field}:exclude` : field; } -/** A stable identity for a committed condition — one pill, one key. */ -export function filterKey(filter: Pick): string { - return slotKey(filter.field, filter.operator); +/** + * A pill's React key. A field's FIRST pill is keyed by the field alone, so a + * pill whose operator changes ("includes all" → "includes none") is rewritten + * in place rather than remounted with an exit animation; only a tag's SECOND + * pill — the exclude beside an include — gets a key of its own. + */ +export function filterPillKey(filter: Filter, filters: readonly Filter[]): string { + const first = filters.find((f) => f.field === filter.field); + return first === filter ? filter.field : slotKey(filter.field, filter.operator); } /** diff --git a/supabase/byod/bootstrap.sql b/supabase/byod/bootstrap.sql index 26836bd21..f082b4dfb 100644 --- a/supabase/byod/bootstrap.sql +++ b/supabase/byod/bootstrap.sql @@ -2,7 +2,7 @@ -- -- ⚠ GENERATED FILE — DO NOT EDIT BY HAND. -- Generated by `node supabase/byod/generate.mjs` from the --- 108 migrations in supabase/migrations/ (newest: 00113_explorer_reverse_activity_filters_and_opened_sort.sql). +-- 109 migrations in supabase/migrations/ (newest: 00114_exclude_tags_dimension.sql). -- CI fails when this file and the migrations disagree; edit a migration (or add -- one) and regenerate instead. -- @@ -22361,6 +22361,990 @@ insert into supabase_migrations.schema_migrations (version, name) values ('00113', 'explorer_reverse_activity_filters_and_opened_sort') on conflict (version) do nothing; +-- ───────────────────────────────────────────────────────────────────────────── +-- 00114_exclude_tags_dimension.sql +-- ───────────────────────────────────────────────────────────────────────────── + +-- ═════════════════════════════════════════════════════════════════════════ +-- 00114 — `exclude_tags`: "tagged a AND NOT tagged b". +-- +-- THE GAP. A label filter is ONE condition: a list plus a mode — `all` +-- (containment), `any` (overlap) or `none` (no overlap). That can say "carries +-- perf", or "carries none of flaky", but never both at once, because the +-- second needs a different mode over a different list. The Explorer's detail +-- panel now offers "filter out" beside "filter in" on every tag chip, and a +-- reader who includes one label and excludes another expects both to hold. +-- +-- THE CHANGE. One appended, null-defaulted parameter on each of the four +-- readers: +-- +-- p_exclude_tags text[] the row carries NONE of these labels +-- +-- It is an independent conjunct — always ANDed, whatever `p_tags_mode` is — +-- and it reuses `lorekit_match_tags(…, 'none')`, so "carries none of" has +-- exactly one definition in the schema: NOT(overlap), never NOT(containment). +-- +-- ── DESIGN ─────────────────────────────────────────────────────────────── +-- +-- NOT A NEW MODE. Folding the exclusion into `p_tags_mode` (an `all_except`, +-- say) would need a second value list on the same parameter, and would leave +-- `any` + exclusion inexpressible. A separate list composes with every mode. +-- +-- THE TAG FACET NEVER SELF-EXCLUDES IT. Self-exclusion (00057) exists so a +-- drilled-in dimension keeps showing its alternatives: the count beside an +-- unselected label is what selecting it would yield. Selecting a label adds it +-- to the INCLUDE condition, and the result still carries none of the excluded +-- labels — so the exclusion belongs in every count, the tag facet's included. +-- It is a match flag there rather than a `base` predicate, so an excluded label +-- stays in the catalog at `count = 0` (00110's rule) and the menu can still +-- offer to un-exclude it. The pivot narrows `base` instead: a matrix emits no +-- zero cells, so the two are equivalent there. +-- +-- A LONE EXCLUSION IS STILL `tags_mode => 'none'`. The dashboard only sends +-- `exclude_tags` when an include condition is ALSO active; on its own an +-- exclusion keeps travelling as it always has, so retention policies (which +-- never gain this parameter) and every existing link mean what they meant. +-- +-- ── SIGNATURES ─────────────────────────────────────────────────────────── +-- +-- `create or replace` cannot change a signature, so each reader is dropped at +-- its EXACT 00113 argument list first and recreated with the parameter +-- appended — §108 and §113 assert exactly one overload of each survives. +-- +-- §114 in `migrations.test.sql` is the executable proof. +-- ═════════════════════════════════════════════════════════════════════════ + +-- ── 1. lorekit_memory_facets (00113 body + p_exclude_tags) ───────────────── + +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, + 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, + -- 00114: labels the row must carry NONE of — an always-applied conjunct, + -- independent of p_tags/p_tags_mode, so "tagged a AND NOT tagged b" is + -- expressible. Appended, null-defaulted: null = not filtered. + p_exclude_tags text[] 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. + -- 00114: `ok_tag` is the WHOLE tag predicate — the include condition AND + -- the exclusion — because that is what every OTHER dimension's count must + -- apply. The tag facet itself self-excludes the include condition (or + -- folds it back in for co-occurrence) but never the exclusion, so it reads + -- `ok_tag_exclusion` on its own. + (lorekit_match_tags(m.tags, p_tags, p_tags_mode) + and lorekit_match_tags(m.tags, p_exclude_tags, 'none')) as ok_tag, + lorekit_match_tags(m.tags, p_exclude_tags, 'none') as ok_tag_exclusion, + 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) + -- 00114: an excluded label is never self-excluded. Selecting another + -- label still yields only rows carrying none of the excluded ones, + -- and an excluded label stays listed at `count = 0` (00110's rule) + -- rather than vanishing from the menu. + and b.ok_tag_exclusion) 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, + text[]) 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, + text[]) 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, + text[]) 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. p_exclude_tags (00114) is an always-applied conjunct on + every count, the tag facet''s included: never self-excluded, so an excluded + label stays listed at count = 0.'; + +-- ── 2. lorekit_memory_activity (00113 body + p_exclude_tags) ─────────────── + +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, + 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, + -- 00114: labels the row must carry NONE of — an always-applied conjunct, + -- independent of p_tags/p_tags_mode, so "tagged a AND NOT tagged b" is + -- expressible. Appended, null-defaulted: null = not filtered. + p_exclude_tags text[] 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_tags(m.tags, p_exclude_tags, 'none') + 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, + text[]) 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, + text[]) 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, + text[]) 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. p_exclude_tags (00114) is applied like every other filter.'; + +-- ── 3. lorekit_memory_pivot (00113 body + p_exclude_tags) ────────────────── + +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, + 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, + -- 00114: labels the row must carry NONE of — an always-applied conjunct, + -- independent of p_tags/p_tags_mode, so "tagged a AND NOT tagged b" is + -- expressible. Appended, null-defaulted: null = not filtered. + p_exclude_tags text[] 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 + ) + -- 00114: in the population, not an axis-excludable `ok_*` flag — with a + -- tag axis an excluded label must still yield no cell, and a matrix + -- emits no zero cells, so narrowing `base` is the exact equivalent. + and lorekit_match_tags(m.tags, p_exclude_tags, 'none') + ) + 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, + text[]) 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, + text[]) 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, + text[]) 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) and p_exclude_tags (00114) are not self-excluded + — they narrow the population, matching lorekit_memory_facets.'; + +-- ── 4. lorekit_memory_list (00113 body + p_exclude_tags) ─────────────────── + +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, + 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, + -- 00114: labels the row must carry NONE of — an always-applied conjunct, + -- independent of p_tags/p_tags_mode, so "tagged a AND NOT tagged b" is + -- expressible. Appended, null-defaulted: null = not filtered. + p_exclude_tags text[] 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_tags(m.tags, $45, 'none') + 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, + p_exclude_tags; +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, + text[]) 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, + text[]) 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, + text[]) 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. p_exclude_tags (00114): the row carries + none of these labels, ANDed with whatever p_tags/p_tags_mode ask.'; +; +insert into supabase_migrations.schema_migrations (version, name) +values ('00114', 'exclude_tags_dimension') +on conflict (version) do nothing; + -- ───────────────────────────────────────────────────────────────────────────── -- storage-overlay.sql — BYOD only: drop foreign keys into auth.users -- ───────────────────────────────────────────────────────────────────────────── From 98a9385e5f2d0e11a1be054b1be5b2acad3d136f Mon Sep 17 00:00:00 2001 From: "mads.thines" Date: Tue, 20 Oct 2026 18:45:19 +0000 Subject: [PATCH 8/9] feat(web): funnel plus/minus icons in the filter flyout, slightly smaller Magnifiers read as search/zoom; the flyout acts on the filter bar. lucide has no funnel-minus, so it is FunnelPlus without the vertical stroke. Glyphs drop from 14px to 12px; buttons keep the 24px target size. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> --- .../src/components/ui/FilterableMetaValue.tsx | 38 ++++++++++++++----- packages/web/src/content/docs/tags.mdx | 2 +- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/packages/web/src/components/ui/FilterableMetaValue.tsx b/packages/web/src/components/ui/FilterableMetaValue.tsx index 19f4be6d1..f26cf3fb5 100644 --- a/packages/web/src/components/ui/FilterableMetaValue.tsx +++ b/packages/web/src/components/ui/FilterableMetaValue.tsx @@ -8,11 +8,12 @@ * * ## Two icons, each explained by its own tooltip * - * The flyout is a compact pair of icon buttons — magnifier-plus "filter in", - * magnifier-minus "filter out", the pair Kibana and Grafana use for "filter for - * value" / "filter out value" — rather than a sentence-long text button. The - * words moved into a second-level {@link Tooltip} on each icon, so the resting - * flyout stays small and the explanation is one hover away. A value that is + * The flyout is a compact pair of icon buttons — funnel-plus "filter in", + * funnel-minus "filter out" — rather than a sentence-long text button. Funnels + * rather than magnifiers: a magnifier reads as "search" or "zoom", and this + * acts on the filter bar, whose own trigger is a filter glyph. The words moved + * into a second-level {@link Tooltip} on each icon, so the resting flyout stays + * small and the explanation is one hover away. A value that is * already filtered in or out shows that button pressed, and its tooltip says * how to take the filter back off (pressing it again does exactly that). * @@ -57,7 +58,7 @@ import { useCallback, useEffect, useLayoutEffect, useRef, useState, type KeyboardEvent, type ReactNode } from 'react'; import { createPortal } from 'react-dom'; -import { ZoomIn, ZoomOut, type LucideIcon } from 'lucide-react'; +import { FunnelPlus, createLucideIcon, type LucideIcon } from 'lucide-react'; import { track } from '@/lib/analytics/track'; import { filterActionLabel, filterAnchorLabel } from '@/lib/filter-from-metadata'; @@ -92,6 +93,23 @@ interface FilterableMetaValueProps { cueOnValue?: boolean; } +/** + * lucide ships `FunnelPlus` and `FunnelX` but no funnel-minus, and `FunnelX` + * reads as "clear the filter" rather than "filter this out". This is + * `FunnelPlus`'s own funnel and horizontal stroke without the vertical one, so + * the pair matches stroke for stroke. + */ +const FunnelMinus = createLucideIcon('funnel-minus', [ + [ + 'path', + { + d: 'M13.354 3H3a1 1 0 0 0-.742 1.67l7.225 7.989A2 2 0 0 1 10 14v6a1 1 0 0 0 .553.895l2 1A1 1 0 0 0 14 21v-7a2 2 0 0 1 .517-1.341l1.218-1.348', + key: 'funnel', + }, + ], + ['path', { d: 'M16 6h6', key: 'minus' }], +]); + /** Matches `Tooltip`/`AnchoredTooltip`'s gap, so the two read as one system. */ const GAP = 6; /** Bridges the visual gap between the value and the floating panel above it. */ @@ -234,17 +252,17 @@ export function FilterableMetaValue({ polarity="include" subject={subject} state={state} - icon={ZoomIn} + icon={FunnelPlus} onActivate={() => activate('include')} /> {onFilterOut && ( <> - + activate('exclude')} /> @@ -301,7 +319,7 @@ function FilterActionButton({ : `text-[var(--color-content-secondary)] hover:bg-[var(--color-bg-raised)] ${tone.hover}`, ].join(' ')} > - + ); diff --git a/packages/web/src/content/docs/tags.mdx b/packages/web/src/content/docs/tags.mdx index 5f2e0a29d..71915a4bf 100644 --- a/packages/web/src/content/docs/tags.mdx +++ b/packages/web/src/content/docs/tags.mdx @@ -99,7 +99,7 @@ Values inside one dimension combine with **OR**; dimensions combine with **AND** Tags are the one dimension that can hold **two pills at once** — one that includes and one that excludes — because a memory carries many tags. "Tag includes all `perf`" **and** "Tag includes none `flaky`" shows memories tagged `perf` that are not tagged `flaky`. Every other dimension has a single value per memory, so its include and exclude collapse into one pill. -You can also filter straight from a memory's detail panel. Hover any tag, kind, host, agent, trigger, repository, branch or pull request and two icons appear: **filter in** (magnifier +) shows only memories with that value, **filter out** (magnifier −) hides them. Hover an icon to see exactly what it will do. The filter is added to whatever is already applied — filtering out a tag while another tag is filtered in gives you both conditions — and a value you have already filtered in or out shows that icon pressed, so clicking it again takes the filter back off. From the keyboard, focus the value and press `Enter` to filter in or `Shift+Enter` to filter out. The scope offers filter in only. +You can also filter straight from a memory's detail panel. Hover any tag, kind, host, agent, trigger, repository, branch or pull request and two icons appear: **filter in** (funnel +) shows only memories with that value, **filter out** (funnel −) hides them. Hover an icon to see exactly what it will do. The filter is added to whatever is already applied — filtering out a tag while another tag is filtered in gives you both conditions — and a value you have already filtered in or out shows that icon pressed, so clicking it again takes the filter back off. From the keyboard, focus the value and press `Enter` to filter in or `Shift+Enter` to filter out. The scope offers filter in only. Two shortcuts worth knowing: From 96eaba26ad73704a3a0599dd2684d1159efcbff3 Mon Sep 17 00:00:00 2001 From: "mads.thines" Date: Tue, 20 Oct 2026 19:03:19 +0000 Subject: [PATCH 9/9] fix(web): instant panel filters and no focus jump from the flyout - useUrlState's optimistic value is now shared by key across instances (url-state-optimistic.ts), so a write from one component shows in every reader of the param at once instead of after the navigation lands. - On /lore the detail panel writes ?filters= / ?scope= through that URL state instead of router.push, so the Explorer updates immediately and every other param (sort, retention, the open lesson) is kept. - The flyout stops click propagation at its portal root and its buttons take no focus on mousedown: React bubbles portal events up the component tree, into TagsField's focus-the-input container. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> --- .../src/components/lore/LessonDetailSheet.tsx | 79 +++++++++------ .../src/components/ui/FilterableMetaValue.tsx | 14 +++ .../components/ui/TagsField.test.stories.tsx | 70 ++++++++++++++ .../web/src/lib/filter-from-metadata.spec.ts | 54 +++++++++++ packages/web/src/lib/filter-from-metadata.ts | 50 +++++++--- .../lib/hooks/url-state-optimistic.spec.ts | 80 ++++++++++++++++ .../web/src/lib/hooks/url-state-optimistic.ts | 96 +++++++++++++++++++ packages/web/src/lib/hooks/useUrlState.ts | 67 ++++++++----- 8 files changed, 448 insertions(+), 62 deletions(-) create mode 100644 packages/web/src/components/ui/TagsField.test.stories.tsx create mode 100644 packages/web/src/lib/hooks/url-state-optimistic.spec.ts create mode 100644 packages/web/src/lib/hooks/url-state-optimistic.ts diff --git a/packages/web/src/components/lore/LessonDetailSheet.tsx b/packages/web/src/components/lore/LessonDetailSheet.tsx index 450f97153..06dd67965 100644 --- a/packages/web/src/components/lore/LessonDetailSheet.tsx +++ b/packages/web/src/components/lore/LessonDetailSheet.tsx @@ -1,7 +1,7 @@ 'use client'; import { useEffect, useMemo, useRef, useState, type KeyboardEvent as ReactKeyboardEvent, type ReactNode } from 'react'; -import { useRouter, useSearchParams } from 'next/navigation'; +import { usePathname, useRouter, useSearchParams } from 'next/navigation'; import { AnimatePresence, motion, useDragControls } from 'motion/react'; import { X, Bot, Zap, Clock, CalendarClock, Archive, RotateCcw, Github, Users, UserCircle, Timer, Layers, Cpu, Repeat, BookOpenCheck, MousePointerClick, Quote } from 'lucide-react'; import { Controller, useWatch, type UseFormReturn } from 'react-hook-form'; @@ -23,9 +23,15 @@ import { FormActionBar } from '@/components/ui/FormActionBar'; import { CONTENT_TABS, CONTENT_TAB_SHORTCUT_KEYS, DEFAULT_CONTENT_TAB, nextTabForKey, shortcutTabForKey, tabAfterSave, type ContentTab } from './content-tabs'; import { tryParseJsonContainer } from '@/lib/json-tree'; import { isTypingTarget } from '@/lib/lesson-list-nav'; -import { metadataFilterTargets, applyMetadataFilterHref, metaValueFilterState, type MetadataFilterTarget } from '@/lib/filter-from-metadata'; +import { + metadataFilterTargets, + applyMetadataFilterHref, + metaValueFilterState, + nextMetadataFilterState, + type MetadataFilterTarget, +} from '@/lib/filter-from-metadata'; import { resolveFilters, valuePolarity, type Filter, type FilterField, type FilterPolarity } from '@/lib/filters'; -import { useUrlState } from '@/lib/hooks/useUrlState'; +import { isPathnameAllowed, useUrlState } from '@/lib/hooks/useUrlState'; import { useEditableForm } from '@/lib/hooks/useEditableForm'; import { useArchiveLesson, useRestoreLesson } from '@/lib/queries/lore'; import type { LessonEntry } from './LessonCard'; @@ -343,17 +349,20 @@ export function LessonDetailSheet({ lesson, onClose, onMutated, layout = 'auto', const queryClient = useQueryClient(); const router = useRouter(); - // R3: applying a metadata filter from the panel is a NAVIGATION — filter - // state lives entirely in the URL (`?filters=`/`?scope=`), and this panel is - // rendered globally by `MemorySidebarProvider`, outside the Explorer's own - // state. So it reads the SAME URL params the Explorer does (mirroring - // `LoreExplorer.tsx`'s own `useUrlState` calls exactly, legacy `?tags=`/ - // `?owner=` included, so a value toggled from here folds in whatever a - // shared legacy link already selected) and computes the next href with the - // pure `applyMetadataFilterHref` (`lib/filter-from-metadata.ts`). + // R3: filter state lives entirely in the URL (`?filters=`/`?scope=`), and + // this panel is rendered globally by `MemorySidebarProvider`, outside the + // Explorer's own state. So it reads the SAME URL params the Explorer does + // (mirroring `LoreExplorer.tsx`'s own `useUrlState` calls exactly — options + // included, so a filter written from here is a Back-button step just like one + // from the menu — and legacy `?tags=`/`?owner=` too, so a value toggled from + // here folds in whatever a shared legacy link already selected). + const pathname = usePathname(); const searchParams = useSearchParams(); - const [scopeParam] = useUrlState('scope', null, { cleanOnPathname: '/lore' }); - const [rawFilters] = useUrlState('filters', null, { cleanOnPathname: '/lore' }); + const [scopeParam, setScopeParam] = useUrlState('scope', null, { cleanOnPathname: '/lore' }); + const [rawFilters, setRawFilters] = useUrlState('filters', null, { + cleanOnPathname: '/lore', + navigationMode: 'push', + }); const [legacyTags] = useUrlState('tags', NO_TAGS, { cleanOnPathname: '/lore' }); const [legacyOwner] = useUrlState('owner', 'all', { cleanOnPathname: '/lore' }); // The COMMITTED search, mirroring `LoreExplorer`'s own non-debounced read of @@ -366,21 +375,35 @@ export function LessonDetailSheet({ lesson, onClose, onMutated, layout = 'auto', ); function handleApplyFilterTarget(target: MetadataFilterTarget, polarity: FilterPolarity = 'include') { - const href = applyMetadataFilterHref( - { - filters: resolvedFilters, - scope: scopeParam, - // `?lesson=`/`?memoryId=` are opaque pass-through strings — - // `searchParams.get` already decodes them, matching what - // `applyMetadataFilterHref` expects (see its own docblock). - lesson: searchParams.get('lesson'), - memoryId: searchParams.get('memoryId'), - q: searchParam, - }, - target, - polarity, - ); - router.push(href); + const current = { + filters: resolvedFilters, + scope: scopeParam, + // `?lesson=`/`?memoryId=` are opaque pass-through strings — + // `searchParams.get` already decodes them, matching what + // `applyMetadataFilterHref` expects (see its own docblock). + lesson: searchParams.get('lesson'), + memoryId: searchParams.get('memoryId'), + q: searchParam, + legacyTags, + legacyOwner, + }; + + // On the Explorer itself, write the one param that changes through the + // same URL state the Explorer reads. `useUrlState` shares a pending write + // across every instance of the key, so the pill, the list query and this + // panel all update at once instead of after the navigation's server round + // trip — and every other param in the URL (sort, retention, the open + // lesson) is left exactly as it was. + if (isPathnameAllowed(pathname, '/lore')) { + const next = nextMetadataFilterState(current, target, polarity); + if (target.kind === 'scope') setScopeParam(next.scope); + else setRawFilters(next.filters); + return; + } + + // Anywhere else the panel can be open, applying a filter means going to the + // Explorer, so it is a real navigation to the `/lore` link it builds. + router.push(applyMetadataFilterHref(current, target, polarity)); } // Wraps a metadata row's VALUE in the hover-reveal filter in / filter out diff --git a/packages/web/src/components/ui/FilterableMetaValue.tsx b/packages/web/src/components/ui/FilterableMetaValue.tsx index f26cf3fb5..70b418ee2 100644 --- a/packages/web/src/components/ui/FilterableMetaValue.tsx +++ b/packages/web/src/components/ui/FilterableMetaValue.tsx @@ -54,6 +54,15 @@ * the DOM (which would be a legitimate but out-of-order stop). The anchor's * accessible name states both keys. The panel's buttons remain a second, * mouse-convenient path to the exact same actions. + * + * ## A portal is outside the DOM, not outside React + * + * React bubbles a portaled element's events up the COMPONENT tree, so a click + * on a flyout button reaches the anchor's ancestors as if it happened inside + * them — `TagsField`'s "click anywhere to focus the input" container among + * them, which moved focus into the tag input on every filter click. The flyout + * stops click propagation at its root, and its buttons take no focus on + * mousedown, so pressing one applies the filter and leaves focus where it was. */ import { useCallback, useEffect, useLayoutEffect, useRef, useState, type KeyboardEvent, type ReactNode } from 'react'; @@ -238,6 +247,8 @@ export function FilterableMetaValue({ data-testid="filterable-meta-value-flyout" role="group" aria-label={`Filter on ${subject}`} + // See "A portal is outside the DOM, not outside React" above. + onClick={(event) => event.stopPropagation()} onMouseEnter={cancelClose} onMouseLeave={scheduleClose} style={pos ? { top: pos.top, left: pos.left } : { top: -9999, left: -9999 }} @@ -310,6 +321,9 @@ function FilterActionButton({ tabIndex={-1} aria-label={explanation} aria-pressed={pressed} + // Keep focus where it was: a mouse press should act, not move focus + // onto a button that is about to fade out. + onMouseDown={(event) => event.preventDefault()} onClick={onActivate} className={[ // 24px — the WCAG 2.2 AA target-size floor this package holds inline actions to. diff --git a/packages/web/src/components/ui/TagsField.test.stories.tsx b/packages/web/src/components/ui/TagsField.test.stories.tsx new file mode 100644 index 000000000..8de7d87f7 --- /dev/null +++ b/packages/web/src/components/ui/TagsField.test.stories.tsx @@ -0,0 +1,70 @@ +import type { Meta, StoryObj } from '@storybook/react'; +import { expect, fn, screen, userEvent, waitFor, within } from 'storybook/test'; + +import { TagsField } from './TagsField'; + +/** + * Interaction tests for the tag chips' filter in / filter out flyout — the + * `/Tests` namespace, tagged `test` and excluded from visual snapshots. + * + * The flyout is PORTALED to `document.body`, but React still bubbles its events + * up the component tree — straight into `TagsField`'s "click anywhere to focus + * the input" container. These pin that pressing a filter button applies the + * filter and leaves focus alone, from the mouse and from the keyboard. + */ +const meta: Meta = { + title: 'UI/TagsField/Tests', + component: TagsField, + tags: ['test'], + parameters: { + chromatic: { disableSnapshot: true }, + layout: 'centered', + }, + args: { + label: 'Tags', + tags: ['perf', 'flaky'], + editable: true, + onChange: fn(), + onTagFilter: fn(), + tagFilterState: () => null, + }, +}; + +export default meta; +type Story = StoryObj; + +export const FilterButtonDoesNotFocusTheInput: Story = { + play: async ({ args, canvasElement, step }) => { + const canvas = within(canvasElement); + const input = canvas.getByRole('textbox'); + + await step('hovering a chip reveals its filter in / filter out flyout', async () => { + await userEvent.hover(canvas.getByRole('button', { name: /^tag "perf"/ })); + const filterOut = await screen.findByRole('button', { name: 'Filter out — hide memories with tag "perf"' }); + const flyout = filterOut.closest('[data-testid="filterable-meta-value-flyout"]'); + await waitFor(() => expect(flyout).toHaveClass('opacity-100')); + }); + + await step('pressing filter out applies it without moving focus into the input', async () => { + await userEvent.click(screen.getByRole('button', { name: 'Filter out — hide memories with tag "perf"' })); + await expect(args.onTagFilter).toHaveBeenCalledWith('perf', 'exclude'); + await expect(input).not.toHaveFocus(); + }); + }, +}; + +export const KeyboardFiltersFromTheChip: Story = { + play: async ({ args, canvasElement, step }) => { + const canvas = within(canvasElement); + const chip = canvas.getByRole('button', { name: /^tag "flaky"/ }); + + await step('Enter filters in and Shift+Enter filters out, focus staying on the chip', async () => { + chip.focus(); + await userEvent.keyboard('{Enter}'); + await expect(args.onTagFilter).toHaveBeenLastCalledWith('flaky', 'include'); + await userEvent.keyboard('{Shift>}{Enter}{/Shift}'); + await expect(args.onTagFilter).toHaveBeenLastCalledWith('flaky', 'exclude'); + await expect(chip).toHaveFocus(); + }); + }, +}; diff --git a/packages/web/src/lib/filter-from-metadata.spec.ts b/packages/web/src/lib/filter-from-metadata.spec.ts index 8632a6b39..bc34f6c0b 100644 --- a/packages/web/src/lib/filter-from-metadata.spec.ts +++ b/packages/web/src/lib/filter-from-metadata.spec.ts @@ -5,6 +5,7 @@ import { filterAnchorLabel, metaValueFilterState, metadataFilterTargets, + nextMetadataFilterState, type CurrentExplorerParams, type MetadataFilterTarget, } from './filter-from-metadata'; @@ -221,6 +222,59 @@ describe('applyMetadataFilterHref — filter out', () => { }); }); +describe('nextMetadataFilterState — the values the panel writes in place on /lore', () => { + const tag = (value: string): MetadataFilterTarget => ({ + kind: 'filter', + field: 'label', + value, + label: `Filter by label "${value}"`, + }); + + it('a filter target changes ?filters= and carries the scope through', () => { + const current: CurrentExplorerParams = { + ...baseParams, + scope: 'project::acme', + filters: [{ field: 'label', operator: 'all', values: ['perf'] }], + }; + expect(nextMetadataFilterState(current, tag('flaky'), 'exclude')).toEqual({ + filters: [ + { field: 'label', operator: 'all', values: ['perf'] }, + { field: 'label', operator: 'nin', values: ['flaky'] }, + ], + scope: 'project::acme', + }); + }); + + it('a scope target changes ?scope= and leaves the filters as they are', () => { + const current: CurrentExplorerParams = { + ...baseParams, + filters: [{ field: 'host', operator: 'in', values: ['aw'] }], + }; + expect( + nextMetadataFilterState(current, { kind: 'scope', scope: 'global', label: 'Filter by scope global' }), + ).toEqual({ filters: [{ field: 'host', operator: 'in', values: ['aw'] }], scope: 'global' }); + }); + + it('removing the last filter drops the param — null, not an empty list', () => { + const current: CurrentExplorerParams = { + ...baseParams, + filters: [{ field: 'label', operator: 'all', values: ['perf'] }], + }; + expect(nextMetadataFilterState(current, tag('perf'), 'include').filters).toBeNull(); + }); + + it('on a legacy ?tags= link, an emptied bar is written explicitly so the old param cannot speak over it', () => { + // Writing in place keeps every other param — `?tags=` included — so the + // empty bar has to be a statement, or the legacy fallback resurrects the pill. + const current: CurrentExplorerParams = { + ...baseParams, + filters: [{ field: 'label', operator: 'all', values: ['perf'] }], + legacyTags: ['perf'], + }; + expect(nextMetadataFilterState(current, tag('perf'), 'include').filters).toEqual([]); + }); +}); + describe('filterActionLabel / filterAnchorLabel', () => { it('explains each button, and how to undo a pressed one', () => { expect(filterActionLabel('include', 'tag "perf"', null)).toBe('Filter in — show only memories with tag "perf"'); diff --git a/packages/web/src/lib/filter-from-metadata.ts b/packages/web/src/lib/filter-from-metadata.ts index ca87e81ac..c47e6c5fd 100644 --- a/packages/web/src/lib/filter-from-metadata.ts +++ b/packages/web/src/lib/filter-from-metadata.ts @@ -192,20 +192,12 @@ export function applyMetadataFilterHref( target: MetadataFilterTarget, polarity: FilterPolarity = 'include', ): string { + const next = nextMetadataFilterState(current, target, polarity); const params = new URLSearchParams(); - if (target.kind === 'scope') { - // `useUrlState`'s wire format — see the field docblock above. - params.set('scope', JSON.stringify(target.scope)); - const filtersValue = filtersParamValue(current.filters, current.legacyTags, current.legacyOwner); - if (filtersValue !== null) params.set('filters', JSON.stringify(filtersValue)); - } else { - const nextFilters = applyValueFilter(current.filters, target.field, target.value, polarity); - const filtersValue = filtersParamValue(nextFilters, current.legacyTags, current.legacyOwner); - if (filtersValue !== null) params.set('filters', JSON.stringify(filtersValue)); - if (current.scope !== null) params.set('scope', JSON.stringify(current.scope)); - } - + // `useUrlState`'s wire format — see the field docblock above. + if (next.scope !== null) params.set('scope', JSON.stringify(next.scope)); + if (next.filters !== null) params.set('filters', JSON.stringify(next.filters)); if (current.lesson !== null) params.set('lesson', current.lesson); if (current.memoryId) params.set('memoryId', current.memoryId); // `useUrlState`'s wire format again — the Explorer reads `?q=` as a JSON @@ -216,6 +208,40 @@ export function applyMetadataFilterHref( return qs ? `/lore?${qs}` : '/lore'; } +/** The `?filters=` and `?scope=` values applying a target produces — `null` meaning "no param". */ +export interface MetadataFilterState { + filters: Filter[] | null; + scope: string | null; +} + +/** + * What applying `target` sets `?filters=` and `?scope=` to — the pure core of + * both ways the panel applies a filter: {@link applyMetadataFilterHref} builds + * a `/lore` link from it when the panel is open elsewhere, and on the Explorer + * itself the panel writes these two values straight into the URL state the + * Explorer reads, so the change shows at once rather than after a navigation. + * + * `filters` is already the PARAM value (`filtersParamValue`): `null` drops the + * param, and `[]` survives only to override a legacy `?tags=` link. + */ +export function nextMetadataFilterState( + current: CurrentExplorerParams, + target: MetadataFilterTarget, + polarity: FilterPolarity = 'include', +): MetadataFilterState { + if (target.kind === 'scope') { + return { + filters: filtersParamValue(current.filters, current.legacyTags, current.legacyOwner), + scope: target.scope, + }; + } + const nextFilters = applyValueFilter(current.filters, target.field, target.value, polarity); + return { + filters: filtersParamValue(nextFilters, current.legacyTags, current.legacyOwner), + scope: current.scope, + }; +} + // ── Active-filter matching (R9 — "which of this memory's values am I already filtering by") ── /** diff --git a/packages/web/src/lib/hooks/url-state-optimistic.spec.ts b/packages/web/src/lib/hooks/url-state-optimistic.spec.ts new file mode 100644 index 000000000..4a420a039 --- /dev/null +++ b/packages/web/src/lib/hooks/url-state-optimistic.spec.ts @@ -0,0 +1,80 @@ +/** + * The shared optimistic layer behind `useUrlState`. + * + * What these pin: a write is visible to EVERY reader of its key at once (the + * detail panel's filter shows in the Explorer before the navigation lands), + * and a pending write can never outlive the reason it existed — the URL + * catching up, the reader leaving the page, or the last reader unmounting. + */ + +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { + UNSET, + readOptimistic, + resetOptimisticForTests, + retainOptimistic, + settleOptimistic, + subscribeOptimistic, + writeOptimistic, +} from './url-state-optimistic'; + +afterEach(() => resetOptimisticForTests()); + +describe('url-state-optimistic', () => { + it('is UNSET until a key is written', () => { + expect(readOptimistic('filters')).toBe(UNSET); + }); + + it('shares one pending value per key, and tells every subscriber', () => { + const explorer = vi.fn(); + const panel = vi.fn(); + const unsubscribeExplorer = subscribeOptimistic(explorer); + const unsubscribePanel = subscribeOptimistic(panel); + + const value = [{ field: 'label', operator: 'nin', values: ['flaky'] }]; + writeOptimistic('filters', value, '/lore'); + + expect(readOptimistic('filters')).toBe(value); + expect(readOptimistic('scope')).toBe(UNSET); + expect(explorer).toHaveBeenCalledTimes(1); + expect(panel).toHaveBeenCalledTimes(1); + unsubscribeExplorer(); + unsubscribePanel(); + }); + + it('returns the same reference on every read, as useSyncExternalStore requires', () => { + writeOptimistic('filters', { a: 1 }, '/lore'); + expect(readOptimistic('filters')).toBe(readOptimistic('filters')); + }); + + it('keeps the value until the URL holds it', () => { + writeOptimistic('filters', ['b'], '/lore'); + settleOptimistic('filters', { pathname: '/lore', urlMatches: false }); + expect(readOptimistic('filters')).toEqual(['b']); + settleOptimistic('filters', { pathname: '/lore', urlMatches: true }); + expect(readOptimistic('filters')).toBe(UNSET); + }); + + it('drops the value once the reader is on another page — it can never land there', () => { + writeOptimistic('filters', ['b'], '/lore'); + settleOptimistic('filters', { pathname: '/insights', urlMatches: false }); + expect(readOptimistic('filters')).toBe(UNSET); + }); + + it('drops the value with the last reader of the key, not before', () => { + const releaseExplorer = retainOptimistic('filters'); + const releasePanel = retainOptimistic('filters'); + writeOptimistic('filters', ['b'], '/lore'); + + releaseExplorer(); + expect(readOptimistic('filters')).toEqual(['b']); + releasePanel(); + expect(readOptimistic('filters')).toBe(UNSET); + }); + + it('a later write replaces an earlier one — the newest click wins', () => { + writeOptimistic('filters', ['a'], '/lore'); + writeOptimistic('filters', ['a', 'b'], '/lore'); + expect(readOptimistic('filters')).toEqual(['a', 'b']); + }); +}); diff --git a/packages/web/src/lib/hooks/url-state-optimistic.ts b/packages/web/src/lib/hooks/url-state-optimistic.ts new file mode 100644 index 000000000..648e7c918 --- /dev/null +++ b/packages/web/src/lib/hooks/url-state-optimistic.ts @@ -0,0 +1,96 @@ +/** + * The optimistic layer behind {@link useUrlState}, shared by URL param KEY. + * + * A URL write through the router lands a navigation later: the new search + * params reach `useSearchParams()` only once the router has finished its cycle, + * which for a dynamic page includes a server round trip. `useUrlState` hides + * that by showing the written value immediately. That layer used to be local + * `useState` per hook INSTANCE, so it only hid the wait from the component that + * wrote — every other component reading the same param kept the old value + * until the navigation landed. Two components reading `?filters=` (the + * Explorer and the detail panel applying a filter into it) disagreed for that + * whole window, and the Explorer looked slow to react to the panel. + * + * So the pending value lives here, keyed by param, and every mounted instance + * reads the same one. Kept free of React (plain subscribe/read/write), so + * `useSyncExternalStore` can sit on top and the rules are unit-testable in the + * node vitest project. + * + * An entry is dropped — the URL becomes the source of truth again — when: + * - the URL now holds the written value (the navigation landed); + * - the pathname differs from the one it was written on (the reader left the + * page the write was aimed at, so it can never land there); + * - the last instance reading the key unmounts (nothing is left to show it, + * and a value surviving that would greet the next page as a stale "pending"). + */ + +/** "No pending value" — a Symbol, because `null` is a legitimate param value. */ +export const UNSET = Symbol('useUrlState/unset'); +export type Unset = typeof UNSET; + +interface PendingWrite { + value: unknown; + pathname: string; +} + +const pending = new Map(); +const readers = new Map(); +const listeners = new Set<() => void>(); + +function notify(): void { + for (const listener of listeners) listener(); +} + +export function subscribeOptimistic(listener: () => void): () => void { + listeners.add(listener); + return () => { + listeners.delete(listener); + }; +} + +/** + * The pending value for `key`, or {@link UNSET}. Returns the stored reference + * itself, so repeated reads are `===` — what `useSyncExternalStore` requires of + * a snapshot. + */ +export function readOptimistic(key: string): unknown { + const entry = pending.get(key); + return entry ? entry.value : UNSET; +} + +export function writeOptimistic(key: string, value: unknown, pathname: string): void { + pending.set(key, { value, pathname }); + notify(); +} + +/** Drop the pending value for `key` if the URL has caught up with it, or can no longer. */ +export function settleOptimistic( + key: string, + { pathname, urlMatches }: { pathname: string; urlMatches: boolean }, +): void { + const entry = pending.get(key); + if (!entry) return; + if (!urlMatches && entry.pathname === pathname) return; + pending.delete(key); + notify(); +} + +/** Count a mounted reader of `key`; the returned release drops its pending value with the last one. */ +export function retainOptimistic(key: string): () => void { + readers.set(key, (readers.get(key) ?? 0) + 1); + return () => { + const left = (readers.get(key) ?? 1) - 1; + if (left > 0) { + readers.set(key, left); + return; + } + readers.delete(key); + if (pending.delete(key)) notify(); + }; +} + +/** Test seam: forget every pending value and reader. */ +export function resetOptimisticForTests(): void { + pending.clear(); + readers.clear(); +} diff --git a/packages/web/src/lib/hooks/useUrlState.ts b/packages/web/src/lib/hooks/useUrlState.ts index 911c2da9c..b9d056e39 100644 --- a/packages/web/src/lib/hooks/useUrlState.ts +++ b/packages/web/src/lib/hooks/useUrlState.ts @@ -23,9 +23,13 @@ * synchronously on every `setState` call, while the URL write happens in the * background. When the URL finally reflects the new value (next render after * navigation), `optimistic` is reset to `UNSET` so the URL is once again the - * source of truth. External URL changes (browser back/forward, direct link - * visits) are reflected immediately because they change `searchParams`, which - * resets the optimistic layer. + * source of truth. + * + * The optimistic value is shared by KEY across every mounted instance + * (`url-state-optimistic.ts`), not held per instance: a write from one + * component (the detail panel adding a filter) shows up at once in every other + * component reading the same param (the Explorer's filter bar and list), rather + * than only after the navigation lands. * * ## Usage (mirrors useState exactly) * @@ -37,18 +41,25 @@ * }); */ -import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import { useCallback, useEffect, useMemo, useRef, useSyncExternalStore } from 'react'; import { usePathname, useRouter, useSearchParams } from 'next/navigation'; +import { + UNSET, + readOptimistic, + retainOptimistic, + settleOptimistic, + subscribeOptimistic, + writeOptimistic, + type Unset, +} from './url-state-optimistic'; // ── Optimistic sentinel ──────────────────────────────────────────────────────── -// Using `null` as "no optimistic value" breaks when T itself is `string | null` -// (e.g. `useUrlState('org', null)`) because a legitimate `null` -// value is indistinguishable from the "not set" sentinel, which causes the -// optimistic layer to never clear and eventually triggers a React invalid-hook -// call (React error #310) on the /settings page. -// A module-scoped Symbol is unique and can never equal a real state value. -const UNSET = Symbol('useUrlState/unset'); -type Unset = typeof UNSET; +// `UNSET` (from `url-state-optimistic.ts`) means "no optimistic value". Using +// `null` breaks when T itself is `string | null` (e.g. +// `useUrlState('org', null)`) because a legitimate `null` value is +// indistinguishable from the "not set" sentinel, which causes the optimistic +// layer to never clear and eventually triggers a React invalid-hook call (React +// error #310) on the /settings page. A Symbol can never equal a real value. export type UrlStateDispatch = (value: S | ((prev: S) => S)) => void; @@ -155,14 +166,22 @@ export function useUrlState( // NOTE: Do NOT use `null` as the sentinel — when T includes `null` (e.g. // `string | null`) the sentinel collides with a legitimate state value, // preventing the optimistic layer from ever clearing (React error #310). - const [optimisticValue, setOptimisticValue] = useState(UNSET); + // Shared by key across instances — see `url-state-optimistic.ts`. The server + // snapshot is always UNSET: nothing has been written before hydration. + const getOptimistic = useCallback(() => readOptimistic(key) as T | Unset, [key]); + const getServerOptimistic = useCallback((): T | Unset => UNSET, []); + const optimisticValue = useSyncExternalStore(subscribeOptimistic, getOptimistic, getServerOptimistic); + + // Count this instance as a reader, so the last one to unmount drops a write + // that never landed instead of leaving it pending for the next page. + useEffect(() => retainOptimistic(key), [key]); - // Once the URL reflects the optimistic value, drop the optimistic layer. + // Once the URL reflects the optimistic value — or the reader has left the + // page it was written on — drop the optimistic layer. useEffect(() => { - if (optimisticValue !== UNSET && serialise(urlValue) === serialise(optimisticValue)) { - setOptimisticValue(UNSET); - } - }, [urlValue, optimisticValue]); + if (optimisticValue === UNSET) return; + settleOptimistic(key, { pathname, urlMatches: serialise(urlValue) === serialise(optimisticValue) }); + }, [key, pathname, urlValue, optimisticValue]); // The value exposed to callers: optimistic takes priority while the URL is // catching up; falls back to the true URL value otherwise. @@ -179,21 +198,25 @@ export function useUrlState( // ── setState ─────────────────────────────────────────────────────────────── const setState = useCallback>( (valueOrUpdater) => { - const prev = optimisticValue !== UNSET ? optimisticValue : deserialise(searchParams.get(key), defaultValue); + // Read the store at call time, not the render's snapshot: another instance + // may have written since this one last rendered. + const pendingValue = readOptimistic(key) as T | Unset; + const prev = pendingValue !== UNSET ? pendingValue : deserialise(searchParams.get(key), defaultValue); const next = typeof valueOrUpdater === 'function' ? (valueOrUpdater as (prev: T) => T)(prev) : valueOrUpdater; - // 1. Optimistic: immediate UI update, no wait for navigation round-trip. - setOptimisticValue(next); + // 1. Optimistic: immediate UI update in every instance reading `key`, no + // wait for the navigation round-trip. + writeOptimistic(key, next, pathname); // 2. Persist: update the URL asynchronously via the router. const url = buildUrl(pathname, searchParams, key, next, defaultValue); navigateRef.current(url, { scroll: false }); }, // eslint-disable-next-line react-hooks/exhaustive-deps - [searchParams, pathname, key, defaultValue, optimisticValue], + [searchParams, pathname, key, defaultValue], ); // ── cleanOnPathname effect ─────────────────────────────────────────────────