From 940ed6449f9f7e7373e6cfd9e1eaf9a0bf13cc59 Mon Sep 17 00:00:00 2001 From: Aaron Knudtson <87577305+knudtty@users.noreply.github.com> Date: Mon, 18 May 2026 15:32:20 -0400 Subject: [PATCH 01/10] feat: emit Lucene filters for KV items direct_read optimization Sidebar and dashboard filters now emit Lucene conditions instead of raw SQL, enabling the KV items direct_read optimization on Map columns (e.g. LogAttributes, ResourceAttributes). This routes filter equality checks through the Lucene serializer's eq() path which rewrites to has(KvItemsColumn, concat(key, sep, value)) when a text(tokenizer=array) index exists. - filtersToQuery emits type:'lucene' for all filters (range stays SQL) - parseLuceneFilter uses the existing Lucene parser + decodeSpecialTokens - Removed stringifyKeys option (Lucene serializer handles JSON toString) - Added AGENTS.md guidance on reusing existing utility functions --- .changeset/filters-direct-read.md | 6 + AGENTS.md | 13 + .../app/src/__tests__/searchFilters.test.ts | 277 ++++++++++++++++-- .../__tests__/useDashboardFilters.test.tsx | 4 +- .../app/src/hooks/useDashboardFilters.tsx | 12 +- packages/app/src/searchFilters.tsx | 16 + .../src/__tests__/filters.test.ts | 228 ++++++++++---- packages/common-utils/src/filters.ts | 116 +++++++- packages/common-utils/src/queryParser.ts | 2 +- 9 files changed, 569 insertions(+), 105 deletions(-) create mode 100644 .changeset/filters-direct-read.md diff --git a/.changeset/filters-direct-read.md b/.changeset/filters-direct-read.md new file mode 100644 index 0000000000..51d8e92311 --- /dev/null +++ b/.changeset/filters-direct-read.md @@ -0,0 +1,6 @@ +--- +"@hyperdx/common-utils": minor +"@hyperdx/app": minor +--- + +feat: emit Lucene conditions from sidebar/dashboard filters to enable KV items direct_read optimization on Map columns diff --git a/AGENTS.md b/AGENTS.md index 90fafcbdb0..c1e02f3ee5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -55,6 +55,19 @@ and lint issues across all packages. Pre-commit hooks handle this when committing, but if you finish edits without committing, run `yarn lint:fix` before stopping. +## Before Writing New Utility Functions + +Before implementing a new parsing, formatting, or transformation function, +search for existing functions that already do the same thing or something close. +Common places to check: + +- `packages/common-utils/src/core/metadata.ts` — key/column parsing utilities + (e.g. `parseKeyPath`) +- `packages/common-utils/src/queryParser.ts` — Lucene parsing (`parse`, + `decodeSpecialTokens`), AST type guards, serialization helpers +- `packages/common-utils/src/filters.ts` — filter state serialization and + parsing (`filtersToQuery`, `parseLuceneFilter`) + ## Key Principles 1. **Multi-tenancy**: All data is scoped to `Team` - ensure proper filtering diff --git a/packages/app/src/__tests__/searchFilters.test.ts b/packages/app/src/__tests__/searchFilters.test.ts index f838f49710..9badbcdf3a 100644 --- a/packages/app/src/__tests__/searchFilters.test.ts +++ b/packages/app/src/__tests__/searchFilters.test.ts @@ -1,5 +1,11 @@ import { enableMapSet } from 'immer'; +import { ClickhouseClient } from '@hyperdx/common-utils/dist/clickhouse/node'; +import { getMetadata } from '@hyperdx/common-utils/dist/core/metadata'; import { filtersToQuery } from '@hyperdx/common-utils/dist/filters'; +import { + CustomSchemaSQLSerializerV2, + SearchQueryBuilder, +} from '@hyperdx/common-utils/dist/queryParser'; import { act, renderHook } from '@testing-library/react'; import { @@ -630,6 +636,133 @@ describe('searchFilters', () => { }); }); + describe('round-trip: Lucene filters', () => { + it('round-trips Map filter with single included value', () => { + const filters = { + "LogAttributes['service.name']": { + included: new Set(['my-app']), + excluded: new Set(), + }, + }; + const query = filtersToQuery(filters); + expect(query).toEqual([ + { + type: 'lucene', + condition: 'LogAttributes.service.name:"my-app"', + }, + ]); + // After round-trip, bracket-notation key becomes dot notation + const parsed = parseQuery(query); + expect(parsed.filters).toEqual({ + 'LogAttributes.service.name': { + included: new Set(['my-app']), + excluded: new Set(), + }, + }); + }); + + it('round-trips Map filter with multiple included values', () => { + const filters = { + "ResourceAttributes['env']": { + included: new Set(['prod', 'staging']), + excluded: new Set(), + }, + }; + const query = filtersToQuery(filters); + const parsed = parseQuery(query); + expect(parsed.filters).toEqual({ + 'ResourceAttributes.env': { + included: new Set(['prod', 'staging']), + excluded: new Set(), + }, + }); + }); + + it('round-trips Map filter with excluded values', () => { + const filters = { + "LogAttributes['level']": { + included: new Set(), + excluded: new Set(['debug', 'trace']), + }, + }; + const query = filtersToQuery(filters); + const parsed = parseQuery(query); + expect(parsed.filters).toEqual({ + 'LogAttributes.level': { + included: new Set(), + excluded: new Set(['debug', 'trace']), + }, + }); + }); + + it('round-trips values containing double quotes', () => { + const filters = { + "LogAttributes['msg']": { + included: new Set(['say "hello"']), + excluded: new Set(), + }, + }; + const query = filtersToQuery(filters); + const parsed = parseQuery(query); + expect(parsed.filters).toEqual({ + 'LogAttributes.msg': { + included: new Set(['say "hello"']), + excluded: new Set(), + }, + }); + }); + + it('round-trips values containing backslashes', () => { + const filters = { + "LogAttributes['path']": { + included: new Set(['C:\\path\\to\\file']), + excluded: new Set(), + }, + }; + const query = filtersToQuery(filters); + const parsed = parseQuery(query); + expect(parsed.filters).toEqual({ + 'LogAttributes.path': { + included: new Set(['C:\\path\\to\\file']), + excluded: new Set(), + }, + }); + }); + + it('round-trips plain key filters', () => { + const filters = { + service: { + included: new Set(['app']), + excluded: new Set(), + }, + }; + const query = filtersToQuery(filters); + const parsed = parseQuery(query); + expect(parsed.filters).toEqual({ + service: { + included: new Set(['app']), + excluded: new Set(), + }, + }); + }); + + it('parses existing lucene filter from URL/API', () => { + const parsed = parseQuery([ + { + type: 'lucene', + condition: + 'LogAttributes.service:"app-1" OR LogAttributes.service:"app-2"', + }, + ]); + expect(parsed.filters).toEqual({ + 'LogAttributes.service': { + included: new Set(['app-1', 'app-2']), + excluded: new Set(), + }, + }); + }); + }); + describe('useSearchPageFilterState', () => { const onFilterChange = jest.fn(); @@ -647,20 +780,15 @@ describe('searchFilters', () => { }); expect(onFilterChange).toHaveBeenLastCalledWith([ - { - type: 'sql', - condition: "service IN ('app')", - }, - { - type: 'sql', - condition: "level IN ('error')", - }, + { type: 'lucene', condition: 'service:"app"' }, + { type: 'lucene', condition: 'level:"error"' }, ]); }); - it('updating filter query', () => { + it('updating filter query from legacy sql input', () => { const { result } = renderHook(() => useSearchPageFilterState({ + // Legacy sql filters are still parsed correctly searchQuery: [ { type: 'sql', condition: `service IN ('hdx-oss-dev-app')` }, { type: 'sql', condition: `hyperdx_event_type IN ('span')` }, @@ -675,10 +803,14 @@ describe('searchFilters', () => { result.current.setFilterValue('another_facet', 'some_value'); }); + // Output is now lucene after round-trip expect(onFilterChange).toHaveBeenLastCalledWith([ - { type: 'sql', condition: `hyperdx_event_type IN ('span')` }, - { type: 'sql', condition: `level IN ('info', 'ok')` }, - { type: 'sql', condition: `another_facet IN ('some_value')` }, + { type: 'lucene', condition: 'hyperdx_event_type:"span"' }, + { + type: 'lucene', + condition: '(level:"info" OR level:"ok")', + }, + { type: 'lucene', condition: 'another_facet:"some_value"' }, ]); }); @@ -699,12 +831,12 @@ describe('searchFilters', () => { }); expect(onFilterChange).toHaveBeenLastCalledWith([ - { type: 'sql', condition: `service IN ('hdx-oss-dev-app')` }, - { type: 'sql', condition: `hyperdx_event_type IN ('span')` }, + { type: 'lucene', condition: 'service:"hdx-oss-dev-app"' }, + { type: 'lucene', condition: 'hyperdx_event_type:"span"' }, ]); }); - it('correctly hydrates filter state from query', () => { + it('correctly hydrates filter state from sql query', () => { const { result } = renderHook(() => useSearchPageFilterState({ searchQuery: [ @@ -745,9 +877,120 @@ describe('searchFilters', () => { }); expect(onFilterChange).toHaveBeenCalledWith([ - { type: 'sql', condition: `service IN ('app')` }, - { type: 'sql', condition: `level IN ('info')` }, // Should only have the included value, no excluded values + { type: 'lucene', condition: 'service:"app"' }, + { type: 'lucene', condition: 'level:"info"' }, ]); }); }); + + describe('filters use direct_read optimization', () => { + const metadata = getMetadata( + new ClickhouseClient({ host: 'http://localhost:8123' }), + ); + + metadata.getColumn = jest.fn().mockImplementation(async ({ column }) => { + if (column === 'LogAttributes') { + return { name: 'LogAttributes', type: 'Map(String, String)' }; + } + return undefined; + }); + metadata.getMaterializedColumnsLookupTable = jest + .fn() + .mockResolvedValue(new Map()); + metadata.getColumns = jest.fn().mockResolvedValue([ + { + name: 'LogAttributes', + type: 'Map(String, String)', + default_type: '', + default_expression: '', + }, + { + name: 'LogAttributeItems', + type: 'Array(String)', + default_type: 'ALIAS', + default_expression: + "arrayMap((arr) -> concat(arr.1, '=', arr.2), LogAttributes::Array(Tuple(String, String)))", + }, + ]); + metadata.getSkipIndices = jest.fn().mockResolvedValue([ + { + name: 'idx_log_attr_items', + type: 'text', + typeFull: 'text(tokenizer=array)', + expression: 'LogAttributeItems', + granularity: 1, + }, + ]); + metadata.getSetting = jest.fn().mockResolvedValue('0'); + + const serializer = new CustomSchemaSQLSerializerV2({ + metadata, + databaseName: 'default', + tableName: 'otel_logs', + connectionId: 'test', + }); + + it('single included filter value uses has() direct_read', async () => { + const filters = filtersToQuery({ + "LogAttributes['service.name']": { + included: new Set(['my-app']), + excluded: new Set(), + }, + }); + + const condition = filters[0]; + expect(condition.type).toBe('lucene'); + const builder = new SearchQueryBuilder( + (condition as { condition: string }).condition, + serializer, + ); + const sql = await builder.build(); + expect(sql).toContain( + "has(`LogAttributeItems`, concat('service.name', '=', 'my-app'))", + ); + }); + + it('multiple included filter values each use has() direct_read', async () => { + const filters = filtersToQuery({ + "LogAttributes['env']": { + included: new Set(['prod', 'staging']), + excluded: new Set(), + }, + }); + + const condition = filters[0]; + expect(condition.type).toBe('lucene'); + const builder = new SearchQueryBuilder( + (condition as { condition: string }).condition, + serializer, + ); + const sql = await builder.build(); + expect(sql).toContain( + "has(`LogAttributeItems`, concat('env', '=', 'prod'))", + ); + expect(sql).toContain( + "has(`LogAttributeItems`, concat('env', '=', 'staging'))", + ); + }); + + it('excluded filter value uses NOT has() direct_read', async () => { + const filters = filtersToQuery({ + "LogAttributes['level']": { + included: new Set(), + excluded: new Set(['debug']), + }, + }); + + const condition = filters[0]; + expect(condition.type).toBe('lucene'); + const builder = new SearchQueryBuilder( + (condition as { condition: string }).condition, + serializer, + ); + const sql = await builder.build(); + expect(sql).toContain( + "NOT has(`LogAttributeItems`, concat('level', '=', 'debug'))", + ); + }); + }); }); diff --git a/packages/app/src/hooks/__tests__/useDashboardFilters.test.tsx b/packages/app/src/hooks/__tests__/useDashboardFilters.test.tsx index baa8904b3b..8f58a5b9bd 100644 --- a/packages/app/src/hooks/__tests__/useDashboardFilters.test.tsx +++ b/packages/app/src/hooks/__tests__/useDashboardFilters.test.tsx @@ -90,7 +90,7 @@ describe('useDashboardFilters', () => { ); }); - it('should generate IN clause for multi-select values', () => { + it('should generate lucene condition for multi-select values', () => { const { result } = renderHook(() => useDashboardFilters(mockFilters)); act(() => { @@ -105,7 +105,7 @@ describe('useDashboardFilters', () => { const query = result2.current.filterQueries[0]; const condition = 'condition' in query ? query.condition : ''; expect(condition).toEqual( - "toString(environment) IN ('production', 'staging')", + '(environment:"production" OR environment:"staging")', ); }); diff --git a/packages/app/src/hooks/useDashboardFilters.tsx b/packages/app/src/hooks/useDashboardFilters.tsx index e1b03c6c40..8a21a22033 100644 --- a/packages/app/src/hooks/useDashboardFilters.tsx +++ b/packages/app/src/hooks/useDashboardFilters.tsx @@ -30,10 +30,7 @@ const useDashboardFilters = (filters: DashboardFilter[]) => { }; } - return filtersToQuery( - filterValues, - { stringifyKeys: false }, // Don't wrap keys with toString(), to preserve exact key names in URL query parameters - ); + return filtersToQuery(filterValues); }); }, [setFilterQueries], @@ -62,12 +59,7 @@ const useDashboardFilters = (filters: DashboardFilter[]) => { return { valuesForExistingFilters, - queriesForExistingFilters: filtersToQuery( - valuesForExistingFilters, - // Wrap keys in `toString()` to support JSON/Dynamic-type columns. - // All keys can be stringified, since filter select values are stringified as well. - { stringifyKeys: true }, - ), + queriesForExistingFilters: filtersToQuery(valuesForExistingFilters), ignoredExpressions: ignored, }; }, [filterQueries, filters]); diff --git a/packages/app/src/searchFilters.tsx b/packages/app/src/searchFilters.tsx index fbb6292857..bbb866351a 100644 --- a/packages/app/src/searchFilters.tsx +++ b/packages/app/src/searchFilters.tsx @@ -3,6 +3,7 @@ import produce from 'immer'; import { type FilterState, filtersToQuery, + parseLuceneFilter, } from '@hyperdx/common-utils/dist/filters'; import type { Filter } from '@hyperdx/common-utils/dist/types'; @@ -307,6 +308,21 @@ export const parseQuery = ( } >(); for (const filter of q) { + if (filter.type === 'lucene') { + const parsedFields = parseLuceneFilter(filter.condition); + if (parsedFields) { + for (const { key, included, excluded } of parsedFields) { + if (!state.has(key)) { + state.set(key, { included: new Set(), excluded: new Set() }); + } + const sets = state.get(key)!; + for (const v of included) sets.included.add(v); + for (const v of excluded) sets.excluded.add(v); + } + } + continue; + } + if (filter.type !== 'sql') continue; // Check for BETWEEN condition (only when BETWEEN appears outside quotes) diff --git a/packages/common-utils/src/__tests__/filters.test.ts b/packages/common-utils/src/__tests__/filters.test.ts index 20253b4c8b..a78478a06b 100644 --- a/packages/common-utils/src/__tests__/filters.test.ts +++ b/packages/common-utils/src/__tests__/filters.test.ts @@ -1,36 +1,33 @@ -import { filtersToQuery } from '@/filters'; +import { filtersToQuery, parseLuceneFilter } from '@/filters'; describe('filters', () => { describe('filtersToQuery', () => { - it('should return empty string when no filters', () => { - const filters = {}; - expect(filtersToQuery(filters)).toEqual([]); + it('should return empty array when no filters', () => { + expect(filtersToQuery({})).toEqual([]); }); - it('should return query for one filter', () => { + it('should emit lucene for a single included value', () => { const filters = { a: { included: new Set(['b']), excluded: new Set() }, }; expect(filtersToQuery(filters)).toEqual([ - { type: 'sql', condition: "a IN ('b')" }, + { type: 'lucene', condition: 'a:"b"' }, ]); }); - it('should return query for multiple filters', () => { + it('should emit lucene with parens for multiple included values', () => { const filters = { - a: { included: new Set(['b']), excluded: new Set() }, c: { included: new Set(['d', 'x']), excluded: new Set(), }, }; expect(filtersToQuery(filters)).toEqual([ - { type: 'sql', condition: "a IN ('b')" }, - { type: 'sql', condition: "c IN ('d', 'x')" }, + { type: 'lucene', condition: '(c:"d" OR c:"x")' }, ]); }); - it('should handle excluded values', () => { + it('should handle excluded values without parens', () => { const filters = { a: { included: new Set(['b']), @@ -38,129 +35,242 @@ describe('filters', () => { }, }; expect(filtersToQuery(filters)).toEqual([ - { type: 'sql', condition: "a IN ('b')" }, - { type: 'sql', condition: "a NOT IN ('c')" }, + { type: 'lucene', condition: 'a:"b"' }, + { type: 'lucene', condition: '-a:"c"' }, ]); }); - it('should wrap keys with toString() when specified', () => { + it('should handle multiple excluded values without parens', () => { const filters = { - 'json.key': { - included: new Set(['value']), - excluded: new Set(['other value']), + a: { + included: new Set(), + excluded: new Set([true, false]), }, }; - expect(filtersToQuery(filters, { stringifyKeys: true })).toEqual([ - { type: 'sql', condition: "toString(json.key) IN ('value')" }, - { type: 'sql', condition: "toString(json.key) NOT IN ('other value')" }, + expect(filtersToQuery(filters)).toEqual([ + { + type: 'lucene', + condition: '-a:"true" AND -a:"false"', + }, ]); }); - it('should should handle boolean filter values', () => { + it('should handle boolean filter values', () => { const filters = { isRootSpan: { included: new Set([true]), excluded: new Set([]), }, - another_column: { - included: new Set([]), - excluded: new Set([true, false]), - }, }; expect(filtersToQuery(filters)).toEqual([ - { type: 'sql', condition: 'isRootSpan IN (true)' }, - { type: 'sql', condition: 'another_column NOT IN (true, false)' }, + { type: 'lucene', condition: 'isRootSpan:"true"' }, ]); }); - it('should escape single quotes in filter values', () => { + it('should escape double quotes in values', () => { const filters = { message: { - included: new Set(["my 'filter' key"]), + included: new Set(['say "hello"']), excluded: new Set(), }, }; expect(filtersToQuery(filters)).toEqual([ - { - type: 'sql', - condition: "message IN ('my ''filter'' key')", - }, + { type: 'lucene', condition: 'message:"say \\"hello\\""' }, ]); }); - it('should escape single quotes in excluded filter values', () => { + it('should pass through single quotes in values', () => { const filters = { message: { - included: new Set(), - excluded: new Set(["it's a test"]), + included: new Set(["it's a test"]), + excluded: new Set(), + }, + }; + expect(filtersToQuery(filters)).toEqual([ + { type: 'lucene', condition: 'message:"it\'s a test"' }, + ]); + }); + + it('should escape backslashes in values', () => { + const filters = { + FilePath: { + included: new Set(['C:\\path\\to\\file']), + excluded: new Set(), }, }; expect(filtersToQuery(filters)).toEqual([ { - type: 'sql', - condition: "message NOT IN ('it''s a test')", + type: 'lucene', + condition: 'FilePath:"C:\\\\path\\\\to\\\\file"', }, ]); }); - it('should escape single quotes with stringifyKeys', () => { + it('should convert Map bracket-notation keys to dot notation', () => { const filters = { - 'json.key': { - included: new Set(["value with 'quotes'"]), + "LogAttributes['service.name']": { + included: new Set(['my-app']), excluded: new Set(), }, }; - expect(filtersToQuery(filters, { stringifyKeys: true })).toEqual([ + expect(filtersToQuery(filters)).toEqual([ { - type: 'sql', - condition: "toString(json.key) IN ('value with ''quotes''')", + type: 'lucene', + condition: 'LogAttributes.service.name:"my-app"', }, ]); }); - it('should escape backslashes in filter values', () => { + it('should handle Map bracket-notation with multiple values', () => { const filters = { - FilePath: { - included: new Set(['C:\\path\\to\\file']), + "LogAttributes['env']": { + included: new Set(['prod', 'staging']), excluded: new Set(), }, }; expect(filtersToQuery(filters)).toEqual([ { - type: 'sql', - condition: "FilePath IN ('C:\\\\path\\\\to\\\\file')", + type: 'lucene', + condition: + '(LogAttributes.env:"prod" OR LogAttributes.env:"staging")', }, ]); }); - it('should escape backslashes in excluded filter values', () => { + it('should handle Map bracket-notation excluded values', () => { const filters = { - FilePath: { + "LogAttributes['level']": { included: new Set(), - excluded: new Set(['C:\\path\\to\\file']), + excluded: new Set(['debug']), }, }; expect(filtersToQuery(filters)).toEqual([ - { - type: 'sql', - condition: "FilePath NOT IN ('C:\\\\path\\\\to\\\\file')", + { type: 'lucene', condition: '-LogAttributes.level:"debug"' }, + ]); + }); + + it('should handle Map bracket-notation with both included and excluded', () => { + const filters = { + "ResourceAttributes['host']": { + included: new Set(['web-1']), + excluded: new Set(['web-2']), }, + }; + expect(filtersToQuery(filters)).toEqual([ + { type: 'lucene', condition: 'ResourceAttributes.host:"web-1"' }, + { type: 'lucene', condition: '-ResourceAttributes.host:"web-2"' }, ]); }); - it('should escape backslashes before single quotes so quotes stay escaped', () => { + it('should keep range as sql', () => { const filters = { - message: { - included: new Set(["a\\'b"]), + duration: { + included: new Set(), excluded: new Set(), + range: { min: 10, max: 500 }, + }, + }; + expect(filtersToQuery(filters)).toEqual([ + { type: 'sql', condition: 'duration BETWEEN 10 AND 500' }, + ]); + }); + + it('should keep range as sql for Map bracket-notation keys', () => { + const filters = { + "LogAttributes['latency']": { + included: new Set(), + excluded: new Set(), + range: { min: 0, max: 100 }, }, }; expect(filtersToQuery(filters)).toEqual([ { type: 'sql', - condition: "message IN ('a\\\\''b')", + condition: "LogAttributes['latency'] BETWEEN 0 AND 100", }, ]); }); }); + + describe('parseLuceneFilter', () => { + it('should return undefined for empty string', () => { + expect(parseLuceneFilter('')).toBeUndefined(); + }); + + it('should return undefined for unquoted terms', () => { + expect(parseLuceneFilter('service:foo')).toBeUndefined(); + }); + + it('should parse a single included term', () => { + expect(parseLuceneFilter('service:"app"')).toEqual([ + { key: 'service', included: ['app'], excluded: [] }, + ]); + }); + + it('should parse a single excluded term', () => { + expect(parseLuceneFilter('-service:"app"')).toEqual([ + { key: 'service', included: [], excluded: ['app'] }, + ]); + }); + + it('should parse multiple included values with OR', () => { + expect(parseLuceneFilter('(service:"app" OR service:"web")')).toEqual([ + { key: 'service', included: ['app', 'web'], excluded: [] }, + ]); + }); + + it('should parse multiple excluded values with AND', () => { + expect( + parseLuceneFilter('-service:"debug" AND -service:"trace"'), + ).toEqual([ + { key: 'service', included: [], excluded: ['debug', 'trace'] }, + ]); + }); + + it('should parse mixed included and excluded for the same field', () => { + expect( + parseLuceneFilter('-service:"bingo" (service:"foo" OR service:"bar")'), + ).toEqual([ + { key: 'service', included: ['foo', 'bar'], excluded: ['bingo'] }, + ]); + }); + + it('should parse multiple distinct fields', () => { + expect(parseLuceneFilter('service:"app" level:"info"')).toEqual([ + { key: 'service', included: ['app'], excluded: [] }, + { key: 'level', included: ['info'], excluded: [] }, + ]); + }); + + it('should parse multiple fields with mixed negation', () => { + expect( + parseLuceneFilter('service:"app" -level:"debug" level:"info"'), + ).toEqual([ + { key: 'service', included: ['app'], excluded: [] }, + { key: 'level', included: ['info'], excluded: ['debug'] }, + ]); + }); + + it('should parse dot-notation fields (Map columns)', () => { + expect(parseLuceneFilter('LogAttributes.env:"prod"')).toEqual([ + { key: 'LogAttributes.env', included: ['prod'], excluded: [] }, + ]); + }); + + it('should handle values with escaped double quotes', () => { + expect(parseLuceneFilter('message:"say \\"hello\\""')).toEqual([ + { key: 'message', included: ['say "hello"'], excluded: [] }, + ]); + }); + + it('should handle values with backslashes', () => { + expect(parseLuceneFilter('path:"C:\\\\dir\\\\file"')).toEqual([ + { key: 'path', included: ['C:\\dir\\file'], excluded: [] }, + ]); + }); + + it('should return undefined for invalid lucene syntax', () => { + expect(parseLuceneFilter('((((')).toBeUndefined(); + }); + }); }); diff --git a/packages/common-utils/src/filters.ts b/packages/common-utils/src/filters.ts index 9a7ac150b1..3e7e29755f 100644 --- a/packages/common-utils/src/filters.ts +++ b/packages/common-utils/src/filters.ts @@ -1,3 +1,7 @@ +import lucene from '@hyperdx/lucene'; + +import { parseKeyPath } from '@/core/metadata'; +import { decodeSpecialTokens, parse } from '@/queryParser'; import { Filter } from '@/types'; export type FilterState = { @@ -8,14 +12,12 @@ export type FilterState = { }; }; -const escapeString = (s: string) => { - return s.replace(/\\/g, '\\\\').replace(/'/g, "''"); +/** Escape a value for use inside a Lucene quoted term ("...") */ +const escapeLuceneQuotedTerm = (s: string) => { + return s.replace(/\\/g, '\\\\').replace(/"/g, '\\"'); }; -export const filtersToQuery = ( - filters: FilterState, - { stringifyKeys = false }: { stringifyKeys?: boolean } = {}, -): Filter[] => { +export const filtersToQuery = (filters: FilterState): Filter[] => { return Object.entries(filters) .filter( ([_, values]) => @@ -25,30 +27,112 @@ export const filtersToQuery = ( ) .flatMap(([key, values]) => { const conditions: Filter[] = []; - const actualKey = stringifyKeys ? `toString(${key})` : key; + const luceneField = parseKeyPath(key).join('.'); if (values.included.size > 0) { + const terms = Array.from(values.included).map( + v => `${luceneField}:"${escapeLuceneQuotedTerm(String(v))}"`, + ); conditions.push({ - type: 'sql' as const, - condition: `${actualKey} IN (${Array.from(values.included) - .map(v => (typeof v === 'string' ? `'${escapeString(v)}'` : v)) - .join(', ')})`, + type: 'lucene' as const, + condition: terms.length > 1 ? `(${terms.join(' OR ')})` : terms[0], }); } if (values.excluded.size > 0) { conditions.push({ - type: 'sql' as const, - condition: `${actualKey} NOT IN (${Array.from(values.excluded) - .map(v => (typeof v === 'string' ? `'${escapeString(v)}'` : v)) - .join(', ')})`, + type: 'lucene' as const, + condition: Array.from(values.excluded) + .map(v => `-${luceneField}:"${escapeLuceneQuotedTerm(String(v))}"`) + .join(' AND '), }); } + if (values.range != null) { conditions.push({ type: 'sql' as const, - condition: `${actualKey} BETWEEN ${values.range.min} AND ${values.range.max}`, + condition: `${key} BETWEEN ${values.range.min} AND ${values.range.max}`, }); } return conditions; }); }; + +// Type guards for lucene AST +function isNodeTerm(node: lucene.Node | lucene.AST): node is lucene.NodeTerm { + return 'term' in node && node.term != null; +} +function isBinaryAST(ast: lucene.AST | lucene.Node): ast is lucene.BinaryAST { + return 'right' in ast && ast.right != null; +} +function isLeftOnlyAST( + ast: lucene.AST | lucene.Node, +): ast is lucene.LeftOnlyAST { + return ( + 'left' in ast && ast.left != null && !('right' in ast && ast.right != null) + ); +} + +/** + * Collect all quoted terms from a Lucene AST into flat {field, value, negated} tuples. + */ +function collectTerms( + ast: lucene.AST | lucene.Node, +): { field: string; value: string; negated: boolean }[] { + if (isNodeTerm(ast)) { + if (!ast.quoted) return []; + const negated = ast.field.startsWith('-'); + const field = negated ? ast.field.slice(1) : ast.field; + return [{ field, value: decodeSpecialTokens(ast.term), negated }]; + } + if (isBinaryAST(ast)) { + return [...collectTerms(ast.left), ...collectTerms(ast.right)]; + } + if (isLeftOnlyAST(ast)) { + return collectTerms(ast.left); + } + return []; +} + +/** + * Parse a Lucene filter condition back into per-field included/excluded values. + * Handles mixed conditions like `-service:"bingo" level:"info" (service:"foo" OR service:"bar")`. + * + * Returns an array with one entry per distinct field, or undefined if parsing fails. + */ +export function parseLuceneFilter(condition: string): + | { + key: string; + included: string[]; + excluded: string[]; + }[] + | undefined { + try { + const ast = parse(condition); + const terms = collectTerms(ast); + if (terms.length === 0) return undefined; + + // Group by field + const byField = new Map< + string, + { included: string[]; excluded: string[] } + >(); + for (const t of terms) { + if (!byField.has(t.field)) { + byField.set(t.field, { included: [], excluded: [] }); + } + const entry = byField.get(t.field)!; + if (t.negated) { + entry.excluded.push(t.value); + } else { + entry.included.push(t.value); + } + } + + return Array.from(byField.entries()).map(([key, vals]) => ({ + key, + ...vals, + })); + } catch { + return undefined; + } +} diff --git a/packages/common-utils/src/queryParser.ts b/packages/common-utils/src/queryParser.ts index 0781019705..37b5554232 100644 --- a/packages/common-utils/src/queryParser.ts +++ b/packages/common-utils/src/queryParser.ts @@ -31,7 +31,7 @@ function encodeSpecialTokens(query: string): string { .replace(/localhost:(\d{1,5})/, 'localhost_COLON_$1') .replace(/\\:/g, 'HDX_COLON'); } -function decodeSpecialTokens(query: string): string { +export function decodeSpecialTokens(query: string): string { return query .replace(/\\"/g, '"') .replace(/HDX_BACKSLASH_LITERAL/g, '\\') From 4ce264d2068e9efb8538c63a35161dfb85ea0794 Mon Sep 17 00:00:00 2001 From: Aaron Knudtson <87577305+knudtty@users.noreply.github.com> Date: Mon, 18 May 2026 16:08:49 -0400 Subject: [PATCH 02/10] fix: pr suggestions --- .../app/src/__tests__/searchFilters.test.ts | 20 ++++++++++++ .../__tests__/useDashboardFilters.test.tsx | 31 +++++++++++++++++++ .../app/src/hooks/useDashboardFilters.tsx | 21 ++++++++++--- .../src/__tests__/filters.test.ts | 9 ++++++ packages/common-utils/src/filters.ts | 20 ++++++++---- 5 files changed, 91 insertions(+), 10 deletions(-) diff --git a/packages/app/src/__tests__/searchFilters.test.ts b/packages/app/src/__tests__/searchFilters.test.ts index 9badbcdf3a..3bb7f22ecf 100644 --- a/packages/app/src/__tests__/searchFilters.test.ts +++ b/packages/app/src/__tests__/searchFilters.test.ts @@ -746,6 +746,26 @@ describe('searchFilters', () => { }); }); + it('round-trips boolean filter values', () => { + const filters = { + isRootSpan: { + included: new Set([true]), + excluded: new Set(), + }, + }; + const query = filtersToQuery(filters); + const parsed = parseQuery(query); + expect(parsed.filters).toEqual({ + isRootSpan: { + included: new Set([true]), + excluded: new Set(), + }, + }); + // Verify .has(true) works (boolean, not string) + expect(parsed.filters.isRootSpan.included.has(true)).toBe(true); + expect(parsed.filters.isRootSpan.included.has('true')).toBe(false); + }); + it('parses existing lucene filter from URL/API', () => { const parsed = parseQuery([ { diff --git a/packages/app/src/hooks/__tests__/useDashboardFilters.test.tsx b/packages/app/src/hooks/__tests__/useDashboardFilters.test.tsx index 8f58a5b9bd..b783dd182a 100644 --- a/packages/app/src/hooks/__tests__/useDashboardFilters.test.tsx +++ b/packages/app/src/hooks/__tests__/useDashboardFilters.test.tsx @@ -272,4 +272,35 @@ describe('useDashboardFilters', () => { expect(result.current.ignoredFilterExpressions).toEqual([]); }); }); + + it('should match bracket-notation expressions after Lucene round-trip', () => { + const bracketFilters: DashboardFilter[] = [ + { + id: 'filter-bracket', + type: 'QUERY_EXPRESSION', + name: 'Pod', + expression: "SpanAttributes['k8s.pod.name']", + source: 'traces', + }, + ]; + + const { result } = renderHook(() => useDashboardFilters(bracketFilters)); + + act(() => { + result.current.setFilterValue("SpanAttributes['k8s.pod.name']", [ + 'pod-1', + ]); + }); + + const { result: result2 } = renderHook(() => + useDashboardFilters(bracketFilters), + ); + + // The bracket-notation expression should still match after the Lucene + // round-trip converts the key to dot notation internally. + expect( + result2.current.filterValues["SpanAttributes['k8s.pod.name']"]?.included, + ).toEqual(new Set(['pod-1'])); + expect(result2.current.ignoredFilterExpressions).toEqual([]); + }); }); diff --git a/packages/app/src/hooks/useDashboardFilters.tsx b/packages/app/src/hooks/useDashboardFilters.tsx index 8a21a22033..ac2e6de9d4 100644 --- a/packages/app/src/hooks/useDashboardFilters.tsx +++ b/packages/app/src/hooks/useDashboardFilters.tsx @@ -1,5 +1,6 @@ import { useCallback, useMemo } from 'react'; import { useQueryState } from 'nuqs'; +import { parseKeyPath } from '@hyperdx/common-utils/dist/core/metadata'; import { FilterState, filtersToQuery, @@ -43,16 +44,28 @@ const useDashboardFilters = (filters: DashboardFilter[]) => { } = useMemo(() => { const { filters: parsedFilters } = parseQuery(filterQueries ?? []); const valuesForExistingFilters: FilterState = {}; - const knownExpressions = new Set(filters.map(f => f.expression)); const ignored: string[] = []; + // Build a normalized lookup so bracket-notation expressions + // (e.g. SpanAttributes['k8s.pod.name']) match the dot-notation keys + // returned by parseLuceneFilter (e.g. SpanAttributes.k8s.pod.name). + const normalizeKey = (k: string) => parseKeyPath(k).join('.'); + const normalizedParsed = new Map( + Object.entries(parsedFilters).map(([k, v]) => [normalizeKey(k), v]), + ); + const knownNormalized = new Set( + filters.map(f => normalizeKey(f.expression)), + ); + for (const { expression } of filters) { - if (expression in parsedFilters) { - valuesForExistingFilters[expression] = parsedFilters[expression]; + const norm = normalizeKey(expression); + const match = normalizedParsed.get(norm); + if (match) { + valuesForExistingFilters[expression] = match; } } for (const key of Object.keys(parsedFilters)) { - if (!knownExpressions.has(key)) { + if (!knownNormalized.has(normalizeKey(key))) { ignored.push(key); } } diff --git a/packages/common-utils/src/__tests__/filters.test.ts b/packages/common-utils/src/__tests__/filters.test.ts index a78478a06b..845325a626 100644 --- a/packages/common-utils/src/__tests__/filters.test.ts +++ b/packages/common-utils/src/__tests__/filters.test.ts @@ -269,6 +269,15 @@ describe('filters', () => { ]); }); + it('should coerce "true"/"false" back to booleans', () => { + expect(parseLuceneFilter('isRootSpan:"true"')).toEqual([ + { key: 'isRootSpan', included: [true], excluded: [] }, + ]); + expect(parseLuceneFilter('-col:"true" AND -col:"false"')).toEqual([ + { key: 'col', included: [], excluded: [true, false] }, + ]); + }); + it('should return undefined for invalid lucene syntax', () => { expect(parseLuceneFilter('((((')).toBeUndefined(); }); diff --git a/packages/common-utils/src/filters.ts b/packages/common-utils/src/filters.ts index 3e7e29755f..7d26db006b 100644 --- a/packages/common-utils/src/filters.ts +++ b/packages/common-utils/src/filters.ts @@ -99,11 +99,18 @@ function collectTerms( * * Returns an array with one entry per distinct field, or undefined if parsing fails. */ +/** Coerce "true"/"false" strings back to booleans, pass through otherwise */ +function coerceBooleanValue(v: string): string | boolean { + if (v === 'true') return true; + if (v === 'false') return false; + return v; +} + export function parseLuceneFilter(condition: string): | { key: string; - included: string[]; - excluded: string[]; + included: (string | boolean)[]; + excluded: (string | boolean)[]; }[] | undefined { try { @@ -111,20 +118,21 @@ export function parseLuceneFilter(condition: string): const terms = collectTerms(ast); if (terms.length === 0) return undefined; - // Group by field + // Group by field, coercing "true"/"false" back to booleans const byField = new Map< string, - { included: string[]; excluded: string[] } + { included: (string | boolean)[]; excluded: (string | boolean)[] } >(); for (const t of terms) { if (!byField.has(t.field)) { byField.set(t.field, { included: [], excluded: [] }); } const entry = byField.get(t.field)!; + const value = coerceBooleanValue(t.value); if (t.negated) { - entry.excluded.push(t.value); + entry.excluded.push(value); } else { - entry.included.push(t.value); + entry.included.push(value); } } From 1cba92556b53374fdd4f24da5c8e7abcb4ac49c3 Mon Sep 17 00:00:00 2001 From: Aaron Knudtson <87577305+knudtty@users.noreply.github.com> Date: Mon, 18 May 2026 16:21:24 -0400 Subject: [PATCH 03/10] fix: update existing tests --- .../src/core/__tests__/linkUrlBuilder.test.ts | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/packages/common-utils/src/core/__tests__/linkUrlBuilder.test.ts b/packages/common-utils/src/core/__tests__/linkUrlBuilder.test.ts index 2dd7ec5fd5..9305dac1c5 100644 --- a/packages/common-utils/src/core/__tests__/linkUrlBuilder.test.ts +++ b/packages/common-utils/src/core/__tests__/linkUrlBuilder.test.ts @@ -243,7 +243,7 @@ describe('renderOnClickSearch', () => { const params = new URLSearchParams(result.url.split('?')[1]); expect( JSON.parse(decodeURIComponent(params.get('filters') ?? '')), - ).toEqual([{ type: 'sql', condition: "ServiceName IN ('MyService')" }]); + ).toEqual([{ type: 'lucene', condition: 'ServiceName:"MyService"' }]); } }); @@ -277,7 +277,9 @@ describe('renderOnClickSearch', () => { const params = new URLSearchParams(result.url.split('?')[1]); expect( JSON.parse(decodeURIComponent(params.get('filters') ?? '')), - ).toEqual([{ type: 'sql', condition: "ServiceName IN ('A', 'B')" }]); + ).toEqual([ + { type: 'lucene', condition: '(ServiceName:"A" OR ServiceName:"B")' }, + ]); } }); @@ -312,8 +314,8 @@ describe('renderOnClickSearch', () => { expect( JSON.parse(decodeURIComponent(params.get('filters') ?? '')), ).toEqual([ - { type: 'sql', condition: "ServiceName IN ('MyService')" }, - { type: 'sql', condition: "SeverityText IN ('error')" }, + { type: 'lucene', condition: 'ServiceName:"MyService"' }, + { type: 'lucene', condition: 'SeverityText:"error"' }, ]); } }); @@ -343,7 +345,7 @@ describe('renderOnClickSearch', () => { const params = new URLSearchParams(result.url.split('?')[1]); expect( JSON.parse(decodeURIComponent(params.get('filters') ?? '')), - ).toEqual([{ type: 'sql', condition: "ServiceName IN ('O''Malley')" }]); + ).toEqual([{ type: 'lucene', condition: 'ServiceName:"O\'Malley"' }]); } }); @@ -373,7 +375,7 @@ describe('renderOnClickSearch', () => { expect( JSON.parse(decodeURIComponent(params.get('filters') ?? '')), ).toEqual([ - { type: 'sql', condition: "FilePath IN ('C:\\\\path\\\\to\\\\file')" }, + { type: 'lucene', condition: 'FilePath:"C:\\\\path\\\\to\\\\file"' }, ]); } }); @@ -405,8 +407,8 @@ describe('renderOnClickSearch', () => { JSON.parse(decodeURIComponent(params.get('filters') ?? '')), ).toEqual([ { - type: 'sql', - condition: "SpanAttributes['url'] IN ('/users%2F42')", + type: 'lucene', + condition: 'SpanAttributes.url:"/users%2F42"', }, ]); } @@ -675,7 +677,7 @@ describe('renderOnClickDashboard', () => { const params = new URLSearchParams(result.url.split('?')[1]); expect( JSON.parse(decodeURIComponent(params.get('filters') ?? '')), - ).toEqual([{ type: 'sql', condition: "ServiceName IN ('MyService')" }]); + ).toEqual([{ type: 'lucene', condition: 'ServiceName:"MyService"' }]); } }); @@ -709,7 +711,9 @@ describe('renderOnClickDashboard', () => { const params = new URLSearchParams(result.url.split('?')[1]); expect( JSON.parse(decodeURIComponent(params.get('filters') ?? '')), - ).toEqual([{ type: 'sql', condition: "ServiceName IN ('A', 'B')" }]); + ).toEqual([ + { type: 'lucene', condition: '(ServiceName:"A" OR ServiceName:"B")' }, + ]); } }); @@ -744,8 +748,8 @@ describe('renderOnClickDashboard', () => { expect( JSON.parse(decodeURIComponent(params.get('filters') ?? '')), ).toEqual([ - { type: 'sql', condition: "ServiceName IN ('MyService')" }, - { type: 'sql', condition: "SeverityText IN ('error')" }, + { type: 'lucene', condition: 'ServiceName:"MyService"' }, + { type: 'lucene', condition: 'SeverityText:"error"' }, ]); } }); @@ -775,7 +779,7 @@ describe('renderOnClickDashboard', () => { const params = new URLSearchParams(result.url.split('?')[1]); expect( JSON.parse(decodeURIComponent(params.get('filters') ?? '')), - ).toEqual([{ type: 'sql', condition: "ServiceName IN ('O''Malley')" }]); + ).toEqual([{ type: 'lucene', condition: 'ServiceName:"O\'Malley"' }]); } }); @@ -805,7 +809,7 @@ describe('renderOnClickDashboard', () => { expect( JSON.parse(decodeURIComponent(params.get('filters') ?? '')), ).toEqual([ - { type: 'sql', condition: "FilePath IN ('C:\\\\path\\\\to\\\\file')" }, + { type: 'lucene', condition: 'FilePath:"C:\\\\path\\\\to\\\\file"' }, ]); } }); @@ -837,8 +841,8 @@ describe('renderOnClickDashboard', () => { JSON.parse(decodeURIComponent(params.get('filters') ?? '')), ).toEqual([ { - type: 'sql', - condition: "SpanAttributes['url'] IN ('/users%2F42')", + type: 'lucene', + condition: 'SpanAttributes.url:"/users%2F42"', }, ]); } From 799f6d8ef319e3715eb2edf82fd80d626294dba4 Mon Sep 17 00:00:00 2001 From: Aaron Knudtson <87577305+knudtty@users.noreply.github.com> Date: Mon, 18 May 2026 16:47:22 -0400 Subject: [PATCH 04/10] fix: e2e testing --- .../e2e/features/dashboard-table-linking.spec.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/app/tests/e2e/features/dashboard-table-linking.spec.ts b/packages/app/tests/e2e/features/dashboard-table-linking.spec.ts index 88c4d43d0e..d4f127b2aa 100644 --- a/packages/app/tests/e2e/features/dashboard-table-linking.spec.ts +++ b/packages/app/tests/e2e/features/dashboard-table-linking.spec.ts @@ -205,10 +205,10 @@ test.describe( const url = new URL(page.url()); expect(url.searchParams.get('source')).toBe(logsSourceId); expect(url.searchParams.get('isLive')).toBe('false'); - // Default row click encodes the group-by filter as a SQL IN clause in - // the JSON-encoded `filters` param. + // Default row click encodes the group-by filter as a Lucene condition + // in the JSON-encoded `filters` param. const filters = url.searchParams.get('filters') ?? '[]'; - expect(filters).toContain(`ServiceName IN ('${serviceName}')`); + expect(filters).toContain(`ServiceName:\\"${serviceName}\\"`); const from = Number(url.searchParams.get('from')); const to = Number(url.searchParams.get('to')); expect(from).toBeGreaterThan(0); @@ -563,8 +563,8 @@ test.describe( const filters = JSON.parse(decodeURIComponent(filtersRaw!)); expect(filters).toEqual([ { - type: 'sql', - condition: `ServiceName IN ('${serviceName}')`, + type: 'lucene', + condition: `ServiceName:"${serviceName}"`, }, ]); }); @@ -649,8 +649,8 @@ test.describe( const filters = JSON.parse(decodeURIComponent(filtersRaw!)); expect(filters).toEqual([ { - type: 'sql', - condition: `ServiceName IN ('${serviceName}')`, + type: 'lucene', + condition: `ServiceName:"${serviceName}"`, }, ]); }); From 9e771469fe3910ace53e1a5ab662e33c57586d66 Mon Sep 17 00:00:00 2001 From: Aaron Knudtson <87577305+knudtty@users.noreply.github.com> Date: Mon, 18 May 2026 17:03:57 -0400 Subject: [PATCH 05/10] fix: filter addition to use common function --- packages/app/src/ChartUtils.tsx | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/packages/app/src/ChartUtils.tsx b/packages/app/src/ChartUtils.tsx index 544c6762d5..46c04b48a3 100644 --- a/packages/app/src/ChartUtils.tsx +++ b/packages/app/src/ChartUtils.tsx @@ -17,6 +17,7 @@ import { getAlignedDateRange, Granularity, } from '@hyperdx/common-utils/dist/core/utils'; +import { filtersToQuery } from '@hyperdx/common-utils/dist/filters'; import { isBuilderChartConfig } from '@hyperdx/common-utils/dist/guards'; import { AggregateFunction as AggFnV2, @@ -923,13 +924,22 @@ export function buildEventsSearchUrl({ // Add group-by column filters if (groupFilters && groupFilters.length > 0) { - groupFilters.forEach(({ column, value }) => { + const filterState: Record< + string, + { included: Set; excluded: Set } + > = {}; + for (const { column, value } of groupFilters) { if (column && value != null) { - // Can't use SQLString.escape here because the search endpoint relies on exist match for UI - const condition = `${column} IN (${SqlString.escape(value)})`; - additionalFilters.push({ type: 'sql', condition }); + if (!filterState[column]) { + filterState[column] = { + included: new Set(), + excluded: new Set(), + }; + } + filterState[column].included.add(String(value)); } - }); + } + additionalFilters.push(...filtersToQuery(filterState)); } // Add Y-axis value range filter (±threshold) for charts From 4a03ee68c2778e7363875be08a207af46228a8bb Mon Sep 17 00:00:00 2001 From: Aaron Knudtson <87577305+knudtty@users.noreply.github.com> Date: Mon, 18 May 2026 18:05:52 -0400 Subject: [PATCH 06/10] fix: normalize filter name for map access --- .../app/src/hooks/useDashboardFilters.tsx | 7 ++-- packages/app/src/searchFilters.tsx | 36 +++++++++++-------- 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/packages/app/src/hooks/useDashboardFilters.tsx b/packages/app/src/hooks/useDashboardFilters.tsx index ac2e6de9d4..8af7e48e89 100644 --- a/packages/app/src/hooks/useDashboardFilters.tsx +++ b/packages/app/src/hooks/useDashboardFilters.tsx @@ -22,10 +22,13 @@ const useDashboardFilters = (filters: DashboardFilter[]) => { (expression: string, values: string[]) => { setFilterQueries(prev => { const { filters: filterValues } = parseQuery(prev ?? []); + // Normalize the expression to dot notation so it matches the keys + // returned by parseQuery (which converts bracket notation to dots). + const key = parseKeyPath(expression).join('.'); if (values.length === 0) { - delete filterValues[expression]; + delete filterValues[key]; } else { - filterValues[expression] = { + filterValues[key] = { included: new Set(values), excluded: new Set(), }; diff --git a/packages/app/src/searchFilters.tsx b/packages/app/src/searchFilters.tsx index bbb866351a..491d020bd6 100644 --- a/packages/app/src/searchFilters.tsx +++ b/packages/app/src/searchFilters.tsx @@ -1,5 +1,6 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import produce from 'immer'; +import { parseKeyPath } from '@hyperdx/common-utils/dist/core/metadata'; import { type FilterState, filtersToQuery, @@ -410,14 +411,17 @@ export const useSearchPageFilterState = ({ value: string | boolean, action?: 'only' | 'exclude' | 'include', ) => { + // Normalize bracket notation to dot notation so the key matches + // what parseQuery returns after a Lucene round-trip. + const key = parseKeyPath(property).join('.'); setFilters(prevFilters => { const newFilters = produce(prevFilters, draft => { - if (!draft[property]) { - draft[property] = { included: new Set(), excluded: new Set() }; + if (!draft[key]) { + draft[key] = { included: new Set(), excluded: new Set() }; } if (action === 'only') { - draft[property] = { + draft[key] = { included: new Set([value]), excluded: new Set(), }; @@ -426,22 +430,22 @@ export const useSearchPageFilterState = ({ if (action === 'exclude') { // Remove from included if it was there - draft[property].included.delete(value); + draft[key].included.delete(value); // Toggle in excluded - if (draft[property].excluded.has(value)) { - draft[property].excluded.delete(value); + if (draft[key].excluded.has(value)) { + draft[key].excluded.delete(value); } else { - draft[property].excluded.add(value); + draft[key].excluded.add(value); } return; } // Regular toggle (include) - draft[property].excluded.delete(value); - if (draft[property].included.has(value)) { - draft[property].included.delete(value); + draft[key].excluded.delete(value); + if (draft[key].included.has(value)) { + draft[key].included.delete(value); } else { - draft[property].included.add(value); + draft[key].included.add(value); } }); updateFilterQuery(newFilters); @@ -453,12 +457,13 @@ export const useSearchPageFilterState = ({ const setFilterRange = useCallback( (property: string, range: { min: number; max: number }) => { + const key = parseKeyPath(property).join('.'); setFilters(prevFilters => { const newFilters = produce(prevFilters, draft => { - if (!draft[property]) { - draft[property] = { included: new Set(), excluded: new Set() }; + if (!draft[key]) { + draft[key] = { included: new Set(), excluded: new Set() }; } - draft[property].range = range; + draft[key].range = range; }); updateFilterQuery(newFilters); return newFilters; @@ -469,9 +474,10 @@ export const useSearchPageFilterState = ({ const clearFilter = useCallback( (property: string) => { + const key = parseKeyPath(property).join('.'); setFilters(prevFilters => { const newFilters = produce(prevFilters, draft => { - delete draft[property]; + delete draft[key]; }); updateFilterQuery(newFilters); return newFilters; From d1ee2ad14aeb9a528fb0ef495c0dda9ae2617844 Mon Sep 17 00:00:00 2001 From: Aaron Knudtson <87577305+knudtty@users.noreply.github.com> Date: Mon, 18 May 2026 18:14:38 -0400 Subject: [PATCH 07/10] fix: PR suggestions --- .../app/src/__tests__/searchFilters.test.ts | 47 ++++ packages/app/src/searchFilters.tsx | 8 +- .../src/__tests__/filters.test.ts | 212 +++++++++++++++++- packages/common-utils/src/filters.ts | 103 ++++++--- 4 files changed, 327 insertions(+), 43 deletions(-) diff --git a/packages/app/src/__tests__/searchFilters.test.ts b/packages/app/src/__tests__/searchFilters.test.ts index 3bb7f22ecf..61cbcebf09 100644 --- a/packages/app/src/__tests__/searchFilters.test.ts +++ b/packages/app/src/__tests__/searchFilters.test.ts @@ -766,6 +766,53 @@ describe('searchFilters', () => { expect(parsed.filters.isRootSpan.included.has('true')).toBe(false); }); + it('round-trips empty string values', () => { + const filters = { + tag: { + included: new Set(['']), + excluded: new Set(), + }, + }; + const query = filtersToQuery(filters); + const parsed = parseQuery(query); + expect(parsed.filters.tag.included).toEqual(new Set([''])); + }); + + it('round-trips values with Lucene reserved characters', () => { + const filters = { + query: { + included: new Set(['(foo) AND [bar]']), + excluded: new Set(), + }, + }; + const query = filtersToQuery(filters); + const parsed = parseQuery(query); + expect(parsed.filters.query.included).toEqual( + new Set(['(foo) AND [bar]']), + ); + }); + + it('round-trips range filters through Lucene', () => { + const filters = { + duration: { + included: new Set(), + excluded: new Set(), + range: { min: 10, max: 500 }, + }, + }; + const query = filtersToQuery(filters); + const parsed = parseQuery(query); + expect(parsed.filters.duration.range).toEqual({ min: 10, max: 500 }); + }); + + it('merges mixed sql and lucene filters for the same key', () => { + const parsed = parseQuery([ + { type: 'sql', condition: `service IN ('app')` }, + { type: 'lucene', condition: 'service:"web"' }, + ]); + expect(parsed.filters.service.included).toEqual(new Set(['app', 'web'])); + }); + it('parses existing lucene filter from URL/API', () => { const parsed = parseQuery([ { diff --git a/packages/app/src/searchFilters.tsx b/packages/app/src/searchFilters.tsx index 491d020bd6..94a4243f80 100644 --- a/packages/app/src/searchFilters.tsx +++ b/packages/app/src/searchFilters.tsx @@ -312,14 +312,20 @@ export const parseQuery = ( if (filter.type === 'lucene') { const parsedFields = parseLuceneFilter(filter.condition); if (parsedFields) { - for (const { key, included, excluded } of parsedFields) { + for (const { key, included, excluded, range } of parsedFields) { if (!state.has(key)) { state.set(key, { included: new Set(), excluded: new Set() }); } const sets = state.get(key)!; for (const v of included) sets.included.add(v); for (const v of excluded) sets.excluded.add(v); + if (range) sets.range = range; } + } else if (filter.condition.trim()) { + console.warn( + 'parseLuceneFilter: could not parse filter condition, filter will be dropped:', + filter.condition, + ); } continue; } diff --git a/packages/common-utils/src/__tests__/filters.test.ts b/packages/common-utils/src/__tests__/filters.test.ts index 845325a626..8ff911fe70 100644 --- a/packages/common-utils/src/__tests__/filters.test.ts +++ b/packages/common-utils/src/__tests__/filters.test.ts @@ -1,4 +1,9 @@ -import { filtersToQuery, parseLuceneFilter } from '@/filters'; +import { type FilterState, filtersToQuery, parseLuceneFilter } from '@/filters'; +import type { Filter } from '@/types'; + +/** Extract condition string from a Filter (filtersToQuery never emits sql_ast) */ +const getCondition = (f: Filter): string => + 'condition' in f ? f.condition : ''; describe('filters', () => { describe('filtersToQuery', () => { @@ -162,7 +167,7 @@ describe('filters', () => { ]); }); - it('should keep range as sql', () => { + it('should emit lucene range syntax', () => { const filters = { duration: { included: new Set(), @@ -171,11 +176,11 @@ describe('filters', () => { }, }; expect(filtersToQuery(filters)).toEqual([ - { type: 'sql', condition: 'duration BETWEEN 10 AND 500' }, + { type: 'lucene', condition: 'duration:[10 TO 500]' }, ]); }); - it('should keep range as sql for Map bracket-notation keys', () => { + it('should emit lucene range with dot notation for Map bracket-notation keys', () => { const filters = { "LogAttributes['latency']": { included: new Set(), @@ -184,10 +189,7 @@ describe('filters', () => { }, }; expect(filtersToQuery(filters)).toEqual([ - { - type: 'sql', - condition: "LogAttributes['latency'] BETWEEN 0 AND 100", - }, + { type: 'lucene', condition: 'LogAttributes.latency:[0 TO 100]' }, ]); }); }); @@ -278,8 +280,202 @@ describe('filters', () => { ]); }); + it('should parse range terms', () => { + expect(parseLuceneFilter('duration:[10 TO 500]')).toEqual([ + { + key: 'duration', + included: [], + excluded: [], + range: { min: 10, max: 500 }, + }, + ]); + }); + + it('should parse range with dot-notation field', () => { + expect(parseLuceneFilter('LogAttributes.latency:[0 TO 100]')).toEqual([ + { + key: 'LogAttributes.latency', + included: [], + excluded: [], + range: { min: 0, max: 100 }, + }, + ]); + }); + it('should return undefined for invalid lucene syntax', () => { expect(parseLuceneFilter('((((')).toBeUndefined(); }); }); + + describe('filtersToQuery -> parseLuceneFilter round-trip', () => { + it('round-trips a single included value', () => { + const state: FilterState = { + service: { + included: new Set(['app']), + excluded: new Set(), + }, + }; + const filters = filtersToQuery(state); + const parsed = parseLuceneFilter(getCondition(filters[0])); + expect(parsed).toEqual([ + { key: 'service', included: ['app'], excluded: [] }, + ]); + }); + + it('round-trips multiple included values', () => { + const state: FilterState = { + env: { + included: new Set(['prod', 'staging']), + excluded: new Set(), + }, + }; + const filters = filtersToQuery(state); + const parsed = parseLuceneFilter(getCondition(filters[0])); + expect(parsed?.[0].key).toBe('env'); + expect(parsed?.[0].included).toEqual( + expect.arrayContaining(['prod', 'staging']), + ); + }); + + it('round-trips excluded values', () => { + const state: FilterState = { + level: { + included: new Set(), + excluded: new Set(['debug', 'trace']), + }, + }; + const filters = filtersToQuery(state); + const parsed = parseLuceneFilter(getCondition(filters[0])); + expect(parsed?.[0].key).toBe('level'); + expect(parsed?.[0].excluded).toEqual( + expect.arrayContaining(['debug', 'trace']), + ); + }); + + it('round-trips boolean values preserving type', () => { + const state: FilterState = { + isRootSpan: { + included: new Set([true]), + excluded: new Set([false]), + }, + }; + const filters = filtersToQuery(state); + const includedParsed = parseLuceneFilter(getCondition(filters[0])); + const excludedParsed = parseLuceneFilter(getCondition(filters[1])); + expect(includedParsed?.[0].included).toEqual([true]); + expect(excludedParsed?.[0].excluded).toEqual([false]); + }); + + it('round-trips the literal string "true" without boolean coercion', () => { + // The string "true" is indistinguishable from boolean true after + // Lucene round-trip due to coerceBooleanValue. This test documents + // that known limitation. + const state: FilterState = { + status: { + included: new Set(['true']), + excluded: new Set(), + }, + }; + const filters = filtersToQuery(state); + const parsed = parseLuceneFilter(getCondition(filters[0])); + // Coerced to boolean — this is the expected (lossy) behavior + expect(parsed?.[0].included).toEqual([true]); + }); + + it('round-trips values with double quotes', () => { + const state: FilterState = { + msg: { + included: new Set(['say "hello"']), + excluded: new Set(), + }, + }; + const filters = filtersToQuery(state); + const parsed = parseLuceneFilter(getCondition(filters[0])); + expect(parsed?.[0].included).toEqual(['say "hello"']); + }); + + it('round-trips values with backslashes', () => { + const state: FilterState = { + path: { + included: new Set(['C:\\dir\\file']), + excluded: new Set(), + }, + }; + const filters = filtersToQuery(state); + const parsed = parseLuceneFilter(getCondition(filters[0])); + expect(parsed?.[0].included).toEqual(['C:\\dir\\file']); + }); + + it('round-trips empty string values', () => { + const state: FilterState = { + tag: { + included: new Set(['']), + excluded: new Set(), + }, + }; + const filters = filtersToQuery(state); + const parsed = parseLuceneFilter(getCondition(filters[0])); + expect(parsed?.[0].included).toEqual(['']); + }); + + it('round-trips values containing Lucene reserved characters', () => { + const state: FilterState = { + query: { + included: new Set(['(foo) AND [bar]']), + excluded: new Set(), + }, + }; + const filters = filtersToQuery(state); + const parsed = parseLuceneFilter(getCondition(filters[0])); + expect(parsed?.[0].included).toEqual(['(foo) AND [bar]']); + }); + + it('round-trips range filters', () => { + const state: FilterState = { + duration: { + included: new Set(), + excluded: new Set(), + range: { min: 10, max: 500 }, + }, + }; + const filters = filtersToQuery(state); + const parsed = parseLuceneFilter(getCondition(filters[0])); + expect(parsed?.[0].range).toEqual({ min: 10, max: 500 }); + }); + + it('round-trips Map bracket-notation key', () => { + const state: FilterState = { + "LogAttributes['service.name']": { + included: new Set(['my-app']), + excluded: new Set(), + }, + }; + const filters = filtersToQuery(state); + const parsed = parseLuceneFilter(getCondition(filters[0])); + // Bracket notation normalizes to dot notation + expect(parsed?.[0].key).toBe('LogAttributes.service.name'); + expect(parsed?.[0].included).toEqual(['my-app']); + }); + + it('round-trips mixed included and excluded for same field', () => { + const state: FilterState = { + env: { + included: new Set(['prod', 'staging']), + excluded: new Set(['dev']), + }, + }; + const filters = filtersToQuery(state); + // filtersToQuery emits separate Filter entries for included/excluded + expect(filters).toHaveLength(2); + // Parse both and merge + const allParsed = filters.flatMap( + f => parseLuceneFilter(getCondition(f)) ?? [], + ); + const envEntries = allParsed.filter(p => p.key === 'env'); + const included = envEntries.flatMap(e => e.included); + const excluded = envEntries.flatMap(e => e.excluded); + expect(included).toEqual(expect.arrayContaining(['prod', 'staging'])); + expect(excluded).toEqual(['dev']); + }); + }); }); diff --git a/packages/common-utils/src/filters.ts b/packages/common-utils/src/filters.ts index 7d26db006b..66b68fa649 100644 --- a/packages/common-utils/src/filters.ts +++ b/packages/common-utils/src/filters.ts @@ -48,9 +48,10 @@ export const filtersToQuery = (filters: FilterState): Filter[] => { } if (values.range != null) { + // Lucene range syntax: field:[min TO max] conditions.push({ - type: 'sql' as const, - condition: `${key} BETWEEN ${values.range.min} AND ${values.range.max}`, + type: 'lucene' as const, + condition: `${luceneField}:[${values.range.min} TO ${values.range.max}]`, }); } return conditions; @@ -61,6 +62,11 @@ export const filtersToQuery = (filters: FilterState): Filter[] => { function isNodeTerm(node: lucene.Node | lucene.AST): node is lucene.NodeTerm { return 'term' in node && node.term != null; } +function isNodeRangedTerm( + node: lucene.Node | lucene.AST, +): node is lucene.NodeRangedTerm { + return 'inclusive' in node && node.inclusive != null; +} function isBinaryAST(ast: lucene.AST | lucene.Node): ast is lucene.BinaryAST { return 'right' in ast && ast.right != null; } @@ -72,33 +78,37 @@ function isLeftOnlyAST( ); } +type CollectedTerm = { field: string; value: string; negated: boolean }; +type CollectedRange = { field: string; min: number; max: number }; + /** - * Collect all quoted terms from a Lucene AST into flat {field, value, negated} tuples. + * Collect quoted terms and range terms from a Lucene AST. */ -function collectTerms( +function collectFromAst( ast: lucene.AST | lucene.Node, -): { field: string; value: string; negated: boolean }[] { + terms: CollectedTerm[], + ranges: CollectedRange[], +): void { if (isNodeTerm(ast)) { - if (!ast.quoted) return []; + if (!ast.quoted) return; const negated = ast.field.startsWith('-'); const field = negated ? ast.field.slice(1) : ast.field; - return [{ field, value: decodeSpecialTokens(ast.term), negated }]; - } - if (isBinaryAST(ast)) { - return [...collectTerms(ast.left), ...collectTerms(ast.right)]; - } - if (isLeftOnlyAST(ast)) { - return collectTerms(ast.left); + terms.push({ field, value: decodeSpecialTokens(ast.term), negated }); + } else if (isNodeRangedTerm(ast)) { + const field = ast.field; + const min = parseFloat(ast.term_min); + const max = parseFloat(ast.term_max); + if (!isNaN(min) && !isNaN(max)) { + ranges.push({ field, min, max }); + } + } else if (isBinaryAST(ast)) { + collectFromAst(ast.left, terms, ranges); + collectFromAst(ast.right, terms, ranges); + } else if (isLeftOnlyAST(ast)) { + collectFromAst(ast.left, terms, ranges); } - return []; } -/** - * Parse a Lucene filter condition back into per-field included/excluded values. - * Handles mixed conditions like `-service:"bingo" level:"info" (service:"foo" OR service:"bar")`. - * - * Returns an array with one entry per distinct field, or undefined if parsing fails. - */ /** Coerce "true"/"false" strings back to booleans, pass through otherwise */ function coerceBooleanValue(v: string): string | boolean { if (v === 'true') return true; @@ -106,28 +116,48 @@ function coerceBooleanValue(v: string): string | boolean { return v; } -export function parseLuceneFilter(condition: string): - | { - key: string; - included: (string | boolean)[]; - excluded: (string | boolean)[]; - }[] - | undefined { +export type ParsedLuceneFilter = { + key: string; + included: (string | boolean)[]; + excluded: (string | boolean)[]; + range?: { min: number; max: number }; +}; + +/** + * Parse a Lucene filter condition back into per-field included/excluded values + * and optional ranges. + * + * Returns an array with one entry per distinct field, or undefined if parsing fails. + */ +export function parseLuceneFilter( + condition: string, +): ParsedLuceneFilter[] | undefined { try { const ast = parse(condition); - const terms = collectTerms(ast); - if (terms.length === 0) return undefined; + const terms: CollectedTerm[] = []; + const ranges: CollectedRange[] = []; + collectFromAst(ast, terms, ranges); + if (terms.length === 0 && ranges.length === 0) return undefined; // Group by field, coercing "true"/"false" back to booleans const byField = new Map< string, - { included: (string | boolean)[]; excluded: (string | boolean)[] } + { + included: (string | boolean)[]; + excluded: (string | boolean)[]; + range?: { min: number; max: number }; + } >(); - for (const t of terms) { - if (!byField.has(t.field)) { - byField.set(t.field, { included: [], excluded: [] }); + + const getEntry = (field: string) => { + if (!byField.has(field)) { + byField.set(field, { included: [], excluded: [] }); } - const entry = byField.get(t.field)!; + return byField.get(field)!; + }; + + for (const t of terms) { + const entry = getEntry(t.field); const value = coerceBooleanValue(t.value); if (t.negated) { entry.excluded.push(value); @@ -136,6 +166,11 @@ export function parseLuceneFilter(condition: string): } } + for (const r of ranges) { + const entry = getEntry(r.field); + entry.range = { min: r.min, max: r.max }; + } + return Array.from(byField.entries()).map(([key, vals]) => ({ key, ...vals, From a442748a7d665297291dcf1d12e27f54309ddcde Mon Sep 17 00:00:00 2001 From: Aaron Knudtson <87577305+knudtty@users.noreply.github.com> Date: Mon, 18 May 2026 19:39:23 -0400 Subject: [PATCH 08/10] fix: allow passthrough filters to allow existing URLs to pass --- packages/api/src/utils/zod.ts | 2 +- packages/app/src/searchFilters.tsx | 22 ++++++++++++++-------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/packages/api/src/utils/zod.ts b/packages/api/src/utils/zod.ts index 6cc1c7abc0..71fb19fc6f 100644 --- a/packages/api/src/utils/zod.ts +++ b/packages/api/src/utils/zod.ts @@ -146,7 +146,7 @@ export type ExternalDashboardFilter = z.infer< >; export const externalDashboardSavedFilterValueSchema = z.object({ - type: z.literal('sql').optional().default('sql'), + type: z.enum(['sql', 'lucene']).optional().default('sql'), condition: z.string().max(10000), }); diff --git a/packages/app/src/searchFilters.tsx b/packages/app/src/searchFilters.tsx index 94a4243f80..bc93c650ab 100644 --- a/packages/app/src/searchFilters.tsx +++ b/packages/app/src/searchFilters.tsx @@ -299,6 +299,8 @@ export const parseQuery = ( q: Filter[], ): { filters: FilterState; + /** Filters that could not be parsed into FilterState (preserved for re-emission) */ + passthroughFilters: Filter[]; } => { const state = new Map< string, @@ -308,6 +310,7 @@ export const parseQuery = ( range?: { min: number; max: number }; } >(); + const passthroughFilters: Filter[] = []; for (const filter of q) { if (filter.type === 'lucene') { const parsedFields = parseLuceneFilter(filter.condition); @@ -322,10 +325,8 @@ export const parseQuery = ( if (range) sets.range = range; } } else if (filter.condition.trim()) { - console.warn( - 'parseLuceneFilter: could not parse filter condition, filter will be dropped:', - filter.condition, - ); + // Preserve unparseable Lucene conditions so they aren't silently lost + passthroughFilters.push(filter); } continue; } @@ -375,7 +376,7 @@ export const parseQuery = ( }); } } - return { filters: Object.fromEntries(state) }; + return { filters: Object.fromEntries(state), passthroughFilters }; }; export const useSearchPageFilterState = ({ @@ -390,7 +391,7 @@ export const useSearchPageFilterState = ({ return parseQuery(searchQuery); } catch (e) { console.error(e); - return { filters: {} }; + return { filters: {}, passthroughFilters: [] }; } }, [searchQuery]); @@ -406,9 +407,14 @@ export const useSearchPageFilterState = ({ const updateFilterQuery = useCallback( (newFilters: FilterState) => { - onFilterChange(filtersToQuery(newFilters)); + // Preserve unparseable filters so they aren't silently lost when + // the user toggles a sidebar facet. + onFilterChange([ + ...filtersToQuery(newFilters), + ...parsedQuery.passthroughFilters, + ]); }, - [onFilterChange], + [onFilterChange, parsedQuery.passthroughFilters], ); const setFilterValue = useCallback( From 8b2122602fbf31fb67e97b1ac3f0ea6e03a0a222 Mon Sep 17 00:00:00 2001 From: Aaron Knudtson <87577305+knudtty@users.noreply.github.com> Date: Mon, 18 May 2026 19:53:57 -0400 Subject: [PATCH 09/10] fix: PR suggestions; support old URLs --- .changeset/filters-direct-read.md | 3 ++ packages/app/src/ChartUtils.tsx | 10 ++-- .../app/src/__tests__/searchFilters.test.ts | 54 +++++++++++++++++++ .../__tests__/useDashboardFilters.test.tsx | 46 ++++++++++++++++ .../app/src/hooks/useDashboardFilters.tsx | 16 +++++- packages/app/src/searchFilters.tsx | 26 +++++++-- .../src/__tests__/filters.test.ts | 8 +-- packages/common-utils/src/filters.ts | 34 ++++-------- packages/common-utils/src/queryParser.ts | 12 +++-- 9 files changed, 168 insertions(+), 41 deletions(-) diff --git a/.changeset/filters-direct-read.md b/.changeset/filters-direct-read.md index 51d8e92311..26182d6853 100644 --- a/.changeset/filters-direct-read.md +++ b/.changeset/filters-direct-read.md @@ -4,3 +4,6 @@ --- feat: emit Lucene conditions from sidebar/dashboard filters to enable KV items direct_read optimization on Map columns + +Legacy `type: 'sql'` filters in URLs are automatically migrated to Lucene +on page load. The persisted `DashboardFilter.expression` in MongoDB is unchanged. diff --git a/packages/app/src/ChartUtils.tsx b/packages/app/src/ChartUtils.tsx index 46c04b48a3..a2a9a05c4b 100644 --- a/packages/app/src/ChartUtils.tsx +++ b/packages/app/src/ChartUtils.tsx @@ -17,7 +17,10 @@ import { getAlignedDateRange, Granularity, } from '@hyperdx/common-utils/dist/core/utils'; -import { filtersToQuery } from '@hyperdx/common-utils/dist/filters'; +import { + type FilterState, + filtersToQuery, +} from '@hyperdx/common-utils/dist/filters'; import { isBuilderChartConfig } from '@hyperdx/common-utils/dist/guards'; import { AggregateFunction as AggFnV2, @@ -924,10 +927,7 @@ export function buildEventsSearchUrl({ // Add group-by column filters if (groupFilters && groupFilters.length > 0) { - const filterState: Record< - string, - { included: Set; excluded: Set } - > = {}; + const filterState: FilterState = {}; for (const { column, value } of groupFilters) { if (column && value != null) { if (!filterState[column]) { diff --git a/packages/app/src/__tests__/searchFilters.test.ts b/packages/app/src/__tests__/searchFilters.test.ts index 61cbcebf09..b108556838 100644 --- a/packages/app/src/__tests__/searchFilters.test.ts +++ b/packages/app/src/__tests__/searchFilters.test.ts @@ -828,6 +828,28 @@ describe('searchFilters', () => { }, }); }); + + it('preserves unparseable Lucene filters as passthrough', () => { + const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + const parsed = parseQuery([ + { type: 'lucene', condition: '((((' }, + { type: 'lucene', condition: 'service:"app"' }, + ]); + // Valid filter is parsed + expect(parsed.filters.service.included).toEqual(new Set(['app'])); + // Invalid filter is preserved in passthroughFilters + expect(parsed.passthroughFilters).toEqual([ + { type: 'lucene', condition: '((((' }, + ]); + warnSpy.mockRestore(); + }); + + it('returns empty array for valid but non-quoted Lucene (no passthrough)', () => { + const parsed = parseQuery([{ type: 'lucene', condition: 'service:foo' }]); + // Unquoted term — valid Lucene but not a filter, nothing extracted + expect(Object.keys(parsed.filters)).toHaveLength(0); + expect(parsed.passthroughFilters).toHaveLength(0); + }); }); describe('useSearchPageFilterState', () => { @@ -948,6 +970,38 @@ describe('searchFilters', () => { { type: 'lucene', condition: 'level:"info"' }, ]); }); + + it('migrates legacy SQL filters to Lucene on load', () => { + const onFilterChangeMigrate = jest.fn(); + renderHook(() => + useSearchPageFilterState({ + searchQuery: [ + { type: 'sql', condition: `service IN ('app')` }, + { type: 'sql', condition: `level IN ('info', 'ok')` }, + ], + onFilterChange: onFilterChangeMigrate, + }), + ); + + // Migration fires on mount, converting SQL to Lucene + expect(onFilterChangeMigrate).toHaveBeenCalledWith([ + { type: 'lucene', condition: 'service:"app"' }, + { type: 'lucene', condition: '(level:"info" OR level:"ok")' }, + ]); + }); + + it('does not migrate when filters are already Lucene', () => { + const onFilterChangeNoMigrate = jest.fn(); + renderHook(() => + useSearchPageFilterState({ + searchQuery: [{ type: 'lucene', condition: 'service:"app"' }], + onFilterChange: onFilterChangeNoMigrate, + }), + ); + + // No migration needed — onFilterChange should not be called + expect(onFilterChangeNoMigrate).not.toHaveBeenCalled(); + }); }); describe('filters use direct_read optimization', () => { diff --git a/packages/app/src/hooks/__tests__/useDashboardFilters.test.tsx b/packages/app/src/hooks/__tests__/useDashboardFilters.test.tsx index b783dd182a..2aa490a135 100644 --- a/packages/app/src/hooks/__tests__/useDashboardFilters.test.tsx +++ b/packages/app/src/hooks/__tests__/useDashboardFilters.test.tsx @@ -303,4 +303,50 @@ describe('useDashboardFilters', () => { ).toEqual(new Set(['pod-1'])); expect(result2.current.ignoredFilterExpressions).toEqual([]); }); + + it('should match dot-notation URL key to bracket-notation expression', () => { + const bracketFilters: DashboardFilter[] = [ + { + id: 'filter-bracket', + type: 'QUERY_EXPRESSION', + name: 'Pod', + expression: "SpanAttributes['k8s.pod.name']", + source: 'traces', + }, + ]; + + // Pre-seed URL state with a dot-notation Lucene filter (as would be + // stored after a round-trip through filtersToQuery → parseQuery). + mockState = [ + { + type: 'lucene', + condition: 'SpanAttributes.k8s.pod.name:"pod-1"', + }, + ]; + + const { result } = renderHook(() => useDashboardFilters(bracketFilters)); + + expect( + result.current.filterValues["SpanAttributes['k8s.pod.name']"]?.included, + ).toEqual(new Set(['pod-1'])); + expect(result.current.ignoredFilterExpressions).toEqual([]); + }); + + it('should migrate legacy SQL filters to Lucene on load', () => { + // Pre-seed URL with old-format SQL filters + mockState = [{ type: 'sql', condition: "environment IN ('production')" }]; + + renderHook(() => useDashboardFilters(mockFilters)); + + // Migration should have called setFilterQueries with Lucene format + expect(mockSetState).toHaveBeenCalled(); + const lastCall = + mockSetState.mock.calls[mockSetState.mock.calls.length - 1]; + // setFilterQueries receives an updater or a value; resolve it + const result = + typeof lastCall[0] === 'function' ? lastCall[0](mockState) : lastCall[0]; + expect(result).toEqual([ + { type: 'lucene', condition: 'environment:"production"' }, + ]); + }); }); diff --git a/packages/app/src/hooks/useDashboardFilters.tsx b/packages/app/src/hooks/useDashboardFilters.tsx index 8af7e48e89..60c62ba990 100644 --- a/packages/app/src/hooks/useDashboardFilters.tsx +++ b/packages/app/src/hooks/useDashboardFilters.tsx @@ -1,4 +1,4 @@ -import { useCallback, useMemo } from 'react'; +import { useCallback, useEffect, useMemo, useRef } from 'react'; import { useQueryState } from 'nuqs'; import { parseKeyPath } from '@hyperdx/common-utils/dist/core/metadata'; import { @@ -80,6 +80,20 @@ const useDashboardFilters = (filters: DashboardFilter[]) => { }; }, [filterQueries, filters]); + // Migrate legacy SQL filters in the URL to Lucene on load + const hasMigratedRef = useRef(false); + useEffect(() => { + if (hasMigratedRef.current || !filterQueries) return; + const hasSqlFilters = filterQueries.some( + f => 'condition' in f && f.type === 'sql', + ); + if (hasSqlFilters) { + hasMigratedRef.current = true; + const { filters: parsed } = parseQuery(filterQueries); + setFilterQueries(filtersToQuery(parsed)); + } + }, [filterQueries, setFilterQueries]); + return { filterValues: valuesForExistingFilters, filterQueries: queriesForExistingFilters, diff --git a/packages/app/src/searchFilters.tsx b/packages/app/src/searchFilters.tsx index bc93c650ab..b08c3e2dbe 100644 --- a/packages/app/src/searchFilters.tsx +++ b/packages/app/src/searchFilters.tsx @@ -314,7 +314,12 @@ export const parseQuery = ( for (const filter of q) { if (filter.type === 'lucene') { const parsedFields = parseLuceneFilter(filter.condition); - if (parsedFields) { + if (parsedFields === undefined) { + // Parse failure — preserve so it isn't silently lost + if (filter.condition.trim()) { + passthroughFilters.push(filter); + } + } else { for (const { key, included, excluded, range } of parsedFields) { if (!state.has(key)) { state.set(key, { included: new Set(), excluded: new Set() }); @@ -324,9 +329,6 @@ export const parseQuery = ( for (const v of excluded) sets.excluded.add(v); if (range) sets.range = range; } - } else if (filter.condition.trim()) { - // Preserve unparseable Lucene conditions so they aren't silently lost - passthroughFilters.push(filter); } continue; } @@ -405,6 +407,22 @@ export const useSearchPageFilterState = ({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [parsedQuery.filters]); + // Migrate legacy SQL filters to Lucene on load so URLs become canonical + const hasMigratedRef = useRef(false); + useEffect(() => { + if (hasMigratedRef.current) return; + const hasSqlFilters = searchQuery?.some( + f => 'condition' in f && f.type === 'sql', + ); + if (hasSqlFilters && Object.keys(parsedQuery.filters).length > 0) { + hasMigratedRef.current = true; + onFilterChange([ + ...filtersToQuery(parsedQuery.filters), + ...parsedQuery.passthroughFilters, + ]); + } + }, [searchQuery, parsedQuery, onFilterChange]); + const updateFilterQuery = useCallback( (newFilters: FilterState) => { // Preserve unparseable filters so they aren't silently lost when diff --git a/packages/common-utils/src/__tests__/filters.test.ts b/packages/common-utils/src/__tests__/filters.test.ts index 8ff911fe70..d2ba8ebb2f 100644 --- a/packages/common-utils/src/__tests__/filters.test.ts +++ b/packages/common-utils/src/__tests__/filters.test.ts @@ -195,12 +195,12 @@ describe('filters', () => { }); describe('parseLuceneFilter', () => { - it('should return undefined for empty string', () => { - expect(parseLuceneFilter('')).toBeUndefined(); + it('should return empty array for empty string', () => { + expect(parseLuceneFilter('')).toEqual([]); }); - it('should return undefined for unquoted terms', () => { - expect(parseLuceneFilter('service:foo')).toBeUndefined(); + it('should return empty array for unquoted terms', () => { + expect(parseLuceneFilter('service:foo')).toEqual([]); }); it('should parse a single included term', () => { diff --git a/packages/common-utils/src/filters.ts b/packages/common-utils/src/filters.ts index 66b68fa649..898ec6dbcd 100644 --- a/packages/common-utils/src/filters.ts +++ b/packages/common-utils/src/filters.ts @@ -1,7 +1,14 @@ import lucene from '@hyperdx/lucene'; import { parseKeyPath } from '@/core/metadata'; -import { decodeSpecialTokens, parse } from '@/queryParser'; +import { + decodeSpecialTokens, + isBinaryAST, + isLeftOnlyAST, + isNodeRangedTerm, + isNodeTerm, + parse, +} from '@/queryParser'; import { Filter } from '@/types'; export type FilterState = { @@ -58,26 +65,6 @@ export const filtersToQuery = (filters: FilterState): Filter[] => { }); }; -// Type guards for lucene AST -function isNodeTerm(node: lucene.Node | lucene.AST): node is lucene.NodeTerm { - return 'term' in node && node.term != null; -} -function isNodeRangedTerm( - node: lucene.Node | lucene.AST, -): node is lucene.NodeRangedTerm { - return 'inclusive' in node && node.inclusive != null; -} -function isBinaryAST(ast: lucene.AST | lucene.Node): ast is lucene.BinaryAST { - return 'right' in ast && ast.right != null; -} -function isLeftOnlyAST( - ast: lucene.AST | lucene.Node, -): ast is lucene.LeftOnlyAST { - return ( - 'left' in ast && ast.left != null && !('right' in ast && ast.right != null) - ); -} - type CollectedTerm = { field: string; value: string; negated: boolean }; type CollectedRange = { field: string; min: number; max: number }; @@ -127,7 +114,9 @@ export type ParsedLuceneFilter = { * Parse a Lucene filter condition back into per-field included/excluded values * and optional ranges. * - * Returns an array with one entry per distinct field, or undefined if parsing fails. + * Returns an array with one entry per distinct field (empty array if the input + * is valid Lucene but contains no quoted terms or ranges), or undefined if + * parsing fails entirely. */ export function parseLuceneFilter( condition: string, @@ -137,7 +126,6 @@ export function parseLuceneFilter( const terms: CollectedTerm[] = []; const ranges: CollectedRange[] = []; collectFromAst(ast, terms, ranges); - if (terms.length === 0 && ranges.length === 0) return undefined; // Group by field, coercing "true"/"false" back to booleans const byField = new Map< diff --git a/packages/common-utils/src/queryParser.ts b/packages/common-utils/src/queryParser.ts index 37b5554232..10bf9ac59e 100644 --- a/packages/common-utils/src/queryParser.ts +++ b/packages/common-utils/src/queryParser.ts @@ -59,17 +59,21 @@ function normalizeChExpression(expr: string): string { const IMPLICIT_FIELD = ''; // Type guards for lucene AST types -function isNodeTerm(node: lucene.Node | lucene.AST): node is lucene.NodeTerm { +export function isNodeTerm( + node: lucene.Node | lucene.AST, +): node is lucene.NodeTerm { return 'term' in node && node.term != null; } -function isNodeRangedTerm( +export function isNodeRangedTerm( node: lucene.Node | lucene.AST, ): node is lucene.NodeRangedTerm { return 'inclusive' in node && node.inclusive != null; } -function isBinaryAST(ast: lucene.AST | lucene.Node): ast is lucene.BinaryAST { +export function isBinaryAST( + ast: lucene.AST | lucene.Node, +): ast is lucene.BinaryAST { return 'right' in ast && ast.right != null; } @@ -79,7 +83,7 @@ function hasStart( return 'start' in ast && !!ast.start; } -function isLeftOnlyAST( +export function isLeftOnlyAST( ast: lucene.AST | lucene.Node, ): ast is lucene.LeftOnlyAST { return ( From 76d19b84e2ee33672496bab94c7dd28e4d093d44 Mon Sep 17 00:00:00 2001 From: Aaron Knudtson <87577305+knudtty@users.noreply.github.com> Date: Mon, 18 May 2026 20:42:35 -0400 Subject: [PATCH 10/10] fix: pr suggestions --- .../app/src/hooks/useDashboardFilters.tsx | 4 ++-- packages/app/src/searchFilters.tsx | 22 +++++++++++-------- packages/common-utils/src/filters.ts | 3 ++- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/packages/app/src/hooks/useDashboardFilters.tsx b/packages/app/src/hooks/useDashboardFilters.tsx index 60c62ba990..7aa439465e 100644 --- a/packages/app/src/hooks/useDashboardFilters.tsx +++ b/packages/app/src/hooks/useDashboardFilters.tsx @@ -89,8 +89,8 @@ const useDashboardFilters = (filters: DashboardFilter[]) => { ); if (hasSqlFilters) { hasMigratedRef.current = true; - const { filters: parsed } = parseQuery(filterQueries); - setFilterQueries(filtersToQuery(parsed)); + const { filters: parsed, passthroughFilters } = parseQuery(filterQueries); + setFilterQueries([...filtersToQuery(parsed), ...passthroughFilters]); } }, [filterQueries, setFilterQueries]); diff --git a/packages/app/src/searchFilters.tsx b/packages/app/src/searchFilters.tsx index b08c3e2dbe..cc35e484fd 100644 --- a/packages/app/src/searchFilters.tsx +++ b/packages/app/src/searchFilters.tsx @@ -2,6 +2,7 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import produce from 'immer'; import { parseKeyPath } from '@hyperdx/common-utils/dist/core/metadata'; import { + coerceBooleanValue, type FilterState, filtersToQuery, parseLuceneFilter, @@ -444,6 +445,9 @@ export const useSearchPageFilterState = ({ // Normalize bracket notation to dot notation so the key matches // what parseQuery returns after a Lucene round-trip. const key = parseKeyPath(property).join('.'); + // Normalize "true"/"false" strings to booleans so the value matches + // what parseLuceneFilter returns after a Lucene round-trip. + const normalizedValue = coerceBooleanValue(value); setFilters(prevFilters => { const newFilters = produce(prevFilters, draft => { if (!draft[key]) { @@ -452,7 +456,7 @@ export const useSearchPageFilterState = ({ if (action === 'only') { draft[key] = { - included: new Set([value]), + included: new Set([normalizedValue]), excluded: new Set(), }; return; @@ -460,22 +464,22 @@ export const useSearchPageFilterState = ({ if (action === 'exclude') { // Remove from included if it was there - draft[key].included.delete(value); + draft[key].included.delete(normalizedValue); // Toggle in excluded - if (draft[key].excluded.has(value)) { - draft[key].excluded.delete(value); + if (draft[key].excluded.has(normalizedValue)) { + draft[key].excluded.delete(normalizedValue); } else { - draft[key].excluded.add(value); + draft[key].excluded.add(normalizedValue); } return; } // Regular toggle (include) - draft[key].excluded.delete(value); - if (draft[key].included.has(value)) { - draft[key].included.delete(value); + draft[key].excluded.delete(normalizedValue); + if (draft[key].included.has(normalizedValue)) { + draft[key].included.delete(normalizedValue); } else { - draft[key].included.add(value); + draft[key].included.add(normalizedValue); } }); updateFilterQuery(newFilters); diff --git a/packages/common-utils/src/filters.ts b/packages/common-utils/src/filters.ts index 898ec6dbcd..d53d64a23a 100644 --- a/packages/common-utils/src/filters.ts +++ b/packages/common-utils/src/filters.ts @@ -97,7 +97,8 @@ function collectFromAst( } /** Coerce "true"/"false" strings back to booleans, pass through otherwise */ -function coerceBooleanValue(v: string): string | boolean { +export function coerceBooleanValue(v: string | boolean): string | boolean { + if (typeof v === 'boolean') return v; if (v === 'true') return true; if (v === 'false') return false; return v;