Skip to content

feat: filter in / filter out from the detail panel, with tag exclusions that AND correctly - #711

Merged
mthines merged 9 commits into
mainfrom
feat/explorer-filter-out-tags
Oct 21, 2026
Merged

mthines merged 9 commits into
mainfrom
feat/explorer-filter-out-tags

Conversation

@dash0-dev

@dash0-dev dash0-dev Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Hovering a value in a memory's detail panel now shows two icons, filter in (funnel +) and filter out (funnel −). Each icon has its own tooltip explaining what it does. Filter out adds a NOT condition, and the result is always the existing bar ANDed with the click. For tags this needed a new API capability: one tags list has one mode, so "tagged perf AND NOT tagged flaky" was not expressible. It is now (exclude_tags, migration 00114).

Changes

  • DB (00114): p_exclude_tags text[] appended to lorekit_memory_list / _facets / _activity / _pivot. It is an always-applied "carries none of these" conjunct, reusing lorekit_match_tags(…, 'none'). The tag facet never self-excludes it, so an excluded tag stays listed at count 0. §114 in migrations.test.sql.
  • API: exclude_tags on the GET query and POST body forms of list/facets/activity/pivot, decoded by the shared dimensions.ts (edge mirror synced) and forwarded by the four handlers.
  • Filter model (filters.ts): tags keep an include pill and an exclude pill that both hold. Every other (single-valued) field keeps one pill. applyValueFilter is the panel's in/out:
    • Tags: excluding several tags gives one "includes none of a, b" pill (NOT a AND NOT b). Including a tag next to an exclusion adds a second pill instead of growing the exclusion.
    • Scalars: is not A + filter out X → is not A, X; is A or X + filter out X → is A; is not A + filter in X → is X.
    • A value is never both required and forbidden: switching polarity moves it. For example, excluding the only included value flips is X → is not X.
    • Pressing an already-pressed icon removes that filter.
  • Wire: exclude_tags is sent only beside a tag include. A lone exclusion keeps tags_mode=none, so existing links and retention policies are unchanged.
  • UI: FilterableMetaValue shows the icon pair, the pressed state, and an in/out cue on the value (accent tint, or struck through). Keyboard: Enter = filter in, Shift+Enter = filter out. The scope offers filter in only, because the API has no "not this scope". The filter menu marks excluded values with a minus.
  • Policy handoff: "Create retention policy" is withheld while a tag is both included and excluded, because a policy carries one tags condition. A hand-edited prefill drops the exclusion visibly.
  • Docs: tags.mdx, llms.txt (regenerated), docs/decisions.md, docs/key-files.md.

Verification

  • CI green: Typecheck/Test/Lint (affected), Storybook interaction + visual, Playwright e2e, Edge deno check, migration order, and Integration smoke. Its "Migration DB tests" step ran migrations.test.sql, including the new §114.
  • Locally: web unit tests (incl. new filters-polarity.spec.ts), schemas tests, edge-schema parity, tsc.
  • Manual check on the preview: open a memory with two tags, filter one in and the other out → two Tag pills, and the list shows only memories with the first tag and without the second.
  • Not run: the repo's /polish + review-loop pass. Run /review-loop on this PR before undrafting.

mthines and others added 6 commits October 20, 2026 17:40
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>
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>
Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
… 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>
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>
…ms.txt

Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

The dashboard preview for this PR — redeployed on each push that changes the web app.

Project Deployment Actions Updated
lorekit Ready Ready Preview Oct 20, 2026 7:05pm UTC

Preview always points at this PR's latest commit; Deployment is the immutable build of 96eaba2. · Comment /web-preview to force a redeploy. · Workflow logs

mthines and others added 3 commits October 20, 2026 18:00
…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>
…ller

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>
- 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>
@mthines

mthines commented Oct 20, 2026

Copy link
Copy Markdown
Owner

🟡 1 non-blocking finding

Adds exclude_tags end to end (migration 00114, edge decoding, four handlers) and filter in / filter out on the detail panel; the SQL and wire mapping check out, one scope toggle does not.

1 advisory finding below the confidence bar — see Less certain below.

What changes on merge. 1: Pressed scope filter cannot be removed.
Finding Where
🟡 1 Pressed scope filter cannot be removed
Fix: Clear the scope when it already equals the target: scope: current.scope === target.scope ? null : target.scope.
packages/web/src/lib/filter-from-metadata.ts:235

Fix all with Agent0

Less certain (1) — advisory, below the confidence bar
Finding Where
2 suggestion: docs/key-files.md:148 still says normalizeFilters keeps at most one pill per dimension, and supabase/functions/memories/CLAUDE.md documents the filter params without exclude_tags.
Confidence 84
docs/key-files.md:184
Review details
Gate Details
Description vs. code ⚠️ Description omits the app-wide useUrlState rewrite (shared optimistic store) that every caller now uses.
Prior review feedback ✅ Earlier review comments are resolved.
Documentation ✅ Migration header, decisions.md, tags.mdx, and the llms template explain the new exclusion.
Self-review signals ✅ No debug logs, leftover TODOs, or unreviewed stubs.
Code review ⚠️ 1 non-blocking finding — see inline comments.
Run Details
Mode full · 3956 lines in delta · tier deep · depth checkout
Anomaly ⚠️ 1 prepare-time anomaly (no --state file supplied — routing computed with lastFullSha=none, incrRunsSinceFull=0 (forces the D6 'no prior deep pass on record' trigger every run); pass --state after reading the LoreKit state record for an accurate routing decision)
Checked 24 of 34 changed files read · 2 possible issues → 1 confirmed → 1 posted
Reach 47 changed exports · 59 dependent files · 0 checked · 59 not checked · 7 changed files imported by 11 files, not traced
Found produced 2 → posted inline 1 · cleared 1 · carried forward 0 · deferred 0 · below-bar 1
Severity 🟡 1 medium
CI Integration smoke (local Supabase) and Web — Playwright e2e are still pending at 96eaba2; every other check passed.
Memories 96 indexed · 0 used

Nothing to report — standards (5 docs), measurability (3 paths classified), integrations (not activated), 0 files skipped.

pr-reviewer · commit 96eaba2 · full review · how these findings are produced · updated Oct 20, 2026 7:15pm UTC

if (target.kind === 'scope') {
return {
filters: filtersParamValue(current.filters, current.legacyTags, current.legacyOwner),
scope: target.scope,

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (medium): 🟡 Pressed scope filter cannot be removed (non-blocking)

When ?scope= already equals this scope, the button reads pressed and promises removal, but nextMetadataFilterState writes the same scope again, so the click does nothing.

Evidence: packages/web/src/lib/filter-from-metadata.ts:288 (pressed tooltip promises removal) · packages/web/src/components/lore/LessonDetailSheet.tsx:399 (writes next.scope unchanged)

if (target.kind === 'scope') {
  return {
    filters: filtersParamValue(current.filters, current.legacyTags, current.legacyOwner),
    scope: current.scope === target.scope ? null : target.scope,
  };
}

Fix with Agent0

pr-reviewer · commit 96eaba2

@mthines
mthines marked this pull request as ready for review October 21, 2026 06:58
@mthines
mthines merged commit c846069 into main Oct 21, 2026
20 checks passed
@dash0-dev

dash0-dev Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor Author

Darkplane Automerge did not approve this pull request. Score 40.56 of 100, threshold 75. Blast radius 30, Evidence quality 82, Change footprint 10.

Darkplane Automerge did not approve this pull request. It scored 40.56 of 100, and 75 was required.

  • 🔴 Blast radius scored 30.
    The PR adds a new migration, 00114_exclude_tags_dimension.sql (975 lines), that drops and recreates four core reader functions (facets, activity, list, pivot) with a new parameter. It also mirrors that SQL into supabase/byod/bootstrap.sql and changes the shared schemas, four memories handlers and about a dozen web filter/URL-state files. A revert would need a new forward migration, and function and migration deploys must be ordered. The change is coherent and null-defaulted, and it has tests, but it spans the DB, the edge functions and the UI.
  • 🔴 Change footprint scored 10.
    This is a hand-written feature across 34 files and about 4,000 lines. supabase/migrations/00114_exclude_tags_dimension.sql (975 lines) drops and recreates the four core memory reader functions with a new p_exclude_tags parameter, and supabase/byod/bootstrap.sql duplicates it. The change also reworks filter logic in packages/web/src/lib/filters.ts and useUrlState.ts, and adds a new url-state-optimistic.ts, so it is a novel change to load-bearing code. It does come with tests, but nothing in it is mechanical.
The other criterion
  • 🟢 Evidence quality scored 82.
    Checks pass at the head commit, including typecheck/test/lint, Storybook, Playwright e2e, Integration smoke and edge deno check. The PR adds filters-polarity.spec.ts, url-state-optimistic.spec.ts, §114 in migrations.test.sql, and a Storybook test for TagsField. The automated review found one non-blocking bug: a pressed scope filter in filter-from-metadata.ts can't be removed. It read only 24 of 34 files and did not trace dependents, and the PR notes the repo's own review-loop pass was not run.

An AI evaluator produced these scores from the diff, the repository history, and the pull request text. The pull request text is author-controlled, so treat the scores as signals rather than proof. Reviewed for commit 96eaba2.

Was this decision right? Give feedback

dash0-dev Bot pushed a commit that referenced this pull request Oct 21, 2026
#712 (this PR's base) was squash-merged as ec35d81, and #711 landed beside
it. Resolved as main plus this PR's own delta (8f08fd3..01cf83a) replayed
on top, so the diff against main is exactly the memory.quota change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant