From f3c4030db687e61dbf2d673e052f4a8e3dcf74a3 Mon Sep 17 00:00:00 2001 From: peymanvahidi Date: Wed, 13 May 2026 21:56:32 +0200 Subject: [PATCH 01/19] refactor(control-bar): remove 3D plane selector and projectionPlane plumbing Drops the Plane (XY/XZ/YZ) - - - - - - ` - : null; - })()} -
diff --git a/packages/core/src/components/control-bar/types.ts b/packages/core/src/components/control-bar/types.ts index 26c02fad..f4f8122f 100644 --- a/packages/core/src/components/control-bar/types.ts +++ b/packages/core/src/components/control-bar/types.ts @@ -54,7 +54,6 @@ export interface ScatterplotElementLike extends Element { selectionMode?: boolean; selectionTool?: 'rectangle' | 'lasso'; selectedProteinIds?: string[]; - projectionPlane?: 'xy' | 'xz' | 'yz'; data?: ProtspaceData; filteredProteinIds?: string[]; filtersActive?: boolean; From 1ed4afcebd094a4d43a76758da6b424ca462a225 Mon Sep 17 00:00:00 2001 From: peymanvahidi Date: Wed, 13 May 2026 22:02:57 +0200 Subject: [PATCH 02/19] refactor(scatter-plot): drop projectionPlane property and dead z assignment Removes the projectionPlane Lit property, the xz/yz coordinate remap, the point.z = coords[2] write (a write-only assignment with no readers), and the projectionPlane entries in the _processData change-detection gates. The data-processor call now passes four args; the unused fifth parameter on data-processor itself is removed in a follow-up commit. 3D Parquet inputs now render using coords[0] / coords[1] only, matching the historical default plane ('xy') that the property defaulted to in production. Part of refactor for issue #196. --- .../components/scatter-plot/scatter-plot.ts | 22 ++++--------------- 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/packages/core/src/components/scatter-plot/scatter-plot.ts b/packages/core/src/components/scatter-plot/scatter-plot.ts index 8f82d538..c43acead 100644 --- a/packages/core/src/components/scatter-plot/scatter-plot.ts +++ b/packages/core/src/components/scatter-plot/scatter-plot.ts @@ -60,7 +60,6 @@ export class ProtspaceScatterplot extends LitElement { // Properties @property({ type: Object }) data: VisualizationData | null = null; @property({ type: Number }) selectedProjectionIndex = 0; - @property({ type: String }) projectionPlane: 'xy' | 'xz' | 'yz' = 'xy'; @property({ type: String }) selectedAnnotation = 'family'; @property({ type: Array }) highlightedProteinIds: string[] = []; @property({ type: Array }) selectedProteinIds: string[] = []; @@ -432,15 +431,13 @@ export class ProtspaceScatterplot extends LitElement { !changedProperties.has('data') && !changedProperties.has('filteredProteinIds') && !changedProperties.has('filtersActive') && - !changedProperties.has('selectedProjectionIndex') && - !changedProperties.has('projectionPlane'); + !changedProperties.has('selectedProjectionIndex'); if ( changedProperties.has('data') || changedProperties.has('filteredProteinIds') || changedProperties.has('filtersActive') || - changedProperties.has('selectedProjectionIndex') || - changedProperties.has('projectionPlane') + changedProperties.has('selectedProjectionIndex') ) { this._processData(); this._scheduleQuadtreeRebuild(); @@ -604,7 +601,6 @@ export class ProtspaceScatterplot extends LitElement { this.selectedProjectionIndex, this._isolationMode, this._isolationHistory, - this.projectionPlane, ); } @@ -779,18 +775,8 @@ export class ProtspaceScatterplot extends LitElement { | [number, number] | [number, number, number]; - let xVal = coords[0]; - let yVal = coords[1]; - - if (coords.length === 3) { - point.z = coords[2]; - if (this.projectionPlane === 'xz') { - yVal = coords[2]; - } else if (this.projectionPlane === 'yz') { - xVal = coords[1]; - yVal = coords[2]; - } - } + const xVal = coords[0]; + const yVal = coords[1]; point.x = xVal; point.y = yVal; From 7ae6f743797dab9d1e1cef76a09b171d45be6b27 Mon Sep 17 00:00:00 2001 From: peymanvahidi Date: Wed, 13 May 2026 22:14:56 +0200 Subject: [PATCH 03/19] refactor(data-processor): drop projectionPlane param + xz/yz remap Removes the projectionPlane parameter from DataProcessor.processVisualizationData and the conditional xz/yz coordinate remap. 3D projections now collapse to coords[0] / coords[1] on every code path. PlotDataPoint loses its optional z field; nothing consumed it (scatter-plot's only write was removed in the previous commit). Test changes: - Delete xz and yz plane-mapping tests. - Replace 'preserves z coordinate for 3D projections' with 'drops the z coordinate for 3D projections (rendered as 2D)', pinning the new contract. Part of refactor for issue #196. --- packages/utils/src/types.ts | 1 - .../src/visualization/data-processor.test.ts | 27 +++---------------- .../utils/src/visualization/data-processor.ts | 14 +--------- 3 files changed, 4 insertions(+), 38 deletions(-) diff --git a/packages/utils/src/types.ts b/packages/utils/src/types.ts index f522e057..9bb7a8ab 100644 --- a/packages/utils/src/types.ts +++ b/packages/utils/src/types.ts @@ -63,7 +63,6 @@ export interface PlotDataPoint { id: string; x: number; y: number; - z?: number; originalIndex: number; } diff --git a/packages/utils/src/visualization/data-processor.test.ts b/packages/utils/src/visualization/data-processor.test.ts index 581677d0..c388979b 100644 --- a/packages/utils/src/visualization/data-processor.test.ts +++ b/packages/utils/src/visualization/data-processor.test.ts @@ -24,7 +24,7 @@ describe('DataProcessor.processVisualizationData', () => { expect(result[1]).toEqual({ id: 'p1', x: 3, y: 4, originalIndex: 1 }); }); - it('preserves z coordinate for 3D projections', () => { + it('drops the z coordinate for 3D projections (rendered as 2D)', () => { const data: VisualizationData = { protein_ids: ['p0'], projections: [{ name: 't', data: [[1, 2, 3]] }], @@ -32,29 +32,8 @@ describe('DataProcessor.processVisualizationData', () => { annotation_data: {}, }; const result = DataProcessor.processVisualizationData(data, 0); - expect(result[0]).toEqual({ id: 'p0', x: 1, y: 2, z: 3, originalIndex: 0 }); - }); - - it('maps coordinates to xz plane when projectionPlane is "xz"', () => { - const data: VisualizationData = { - protein_ids: ['p0'], - projections: [{ name: 't', data: [[10, 20, 30]] }], - annotations: {}, - annotation_data: {}, - }; - const result = DataProcessor.processVisualizationData(data, 0, false, undefined, 'xz'); - expect(result[0]).toEqual({ id: 'p0', x: 10, y: 30, z: 30, originalIndex: 0 }); - }); - - it('maps coordinates to yz plane when projectionPlane is "yz"', () => { - const data: VisualizationData = { - protein_ids: ['p0'], - projections: [{ name: 't', data: [[10, 20, 30]] }], - annotations: {}, - annotation_data: {}, - }; - const result = DataProcessor.processVisualizationData(data, 0, false, undefined, 'yz'); - expect(result[0]).toEqual({ id: 'p0', x: 20, y: 30, z: 30, originalIndex: 0 }); + expect(result[0]).toEqual({ id: 'p0', x: 1, y: 2, originalIndex: 0 }); + expect(result[0]).not.toHaveProperty('z'); }); it('returns empty array when projection index is out of range', () => { diff --git a/packages/utils/src/visualization/data-processor.ts b/packages/utils/src/visualization/data-processor.ts index c34ae7f7..47cab637 100644 --- a/packages/utils/src/visualization/data-processor.ts +++ b/packages/utils/src/visualization/data-processor.ts @@ -7,7 +7,6 @@ export class DataProcessor { projectionIndex: number, isolationMode: boolean = false, isolationHistory?: string[][], - projectionPlane: 'xy' | 'xz' | 'yz' = 'xy', ): PlotDataPoint[] { if (!data.projections[projectionIndex]) return []; @@ -16,18 +15,7 @@ export class DataProcessor { | [number, number] | [number, number, number]; - let xVal = coordinates[0]; - let yVal = coordinates[1]; - if (coordinates.length === 3) { - if (projectionPlane === 'xz') { - yVal = coordinates[2]; - } else if (projectionPlane === 'yz') { - xVal = coordinates[1]; - yVal = coordinates[2]; - } - return { id, x: xVal, y: yVal, z: coordinates[2], originalIndex: index }; - } - return { id, x: xVal, y: yVal, originalIndex: index }; + return { id, x: coordinates[0], y: coordinates[1], originalIndex: index }; }); if (isolationMode && isolationHistory && isolationHistory.length > 0) { From 2dbcffc6824a765955acaf0658ae74519bf43f63 Mon Sep 17 00:00:00 2001 From: peymanvahidi Date: Wed, 13 May 2026 22:33:41 +0200 Subject: [PATCH 04/19] refactor(control-bar): drop dead isProjection3D and getProjectionPlane helpers After the plane selector removal in earlier commits, these helpers have zero remaining callers in packages/ or app/. Removed both functions and their describe blocks from the test file. Part of refactor for issue #196. --- .../control-bar/control-bar-helpers.test.ts | 44 ------------------- .../control-bar/control-bar-helpers.ts | 21 --------- 2 files changed, 65 deletions(-) diff --git a/packages/core/src/components/control-bar/control-bar-helpers.test.ts b/packages/core/src/components/control-bar/control-bar-helpers.test.ts index 9b6c1435..d8b6a4ac 100644 --- a/packages/core/src/components/control-bar/control-bar-helpers.test.ts +++ b/packages/core/src/components/control-bar/control-bar-helpers.test.ts @@ -1,8 +1,6 @@ import { describe, it, expect } from 'vitest'; import { EXPORT_DEFAULTS, - isProjection3D, - getProjectionPlane, shouldDisableSelection, getSelectionDisabledMessage, toggleProteinSelection, @@ -92,48 +90,6 @@ describe('control-bar-helpers', () => { }); }); - describe('isProjection3D', () => { - const projectionsMeta = [ - { name: 'projection2D', metadata: { dimension: 2 as const } }, - { name: 'projection3D', metadata: { dimension: 3 as const } }, - { name: 'projectionNoMeta' }, - ]; - - it('returns true for 3D projection', () => { - expect(isProjection3D('projection3D', projectionsMeta)).toBe(true); - }); - - it('returns false for 2D projection', () => { - expect(isProjection3D('projection2D', projectionsMeta)).toBe(false); - }); - - it('returns false for projection without metadata', () => { - expect(isProjection3D('projectionNoMeta', projectionsMeta)).toBe(false); - }); - - it('returns false for non-existent projection', () => { - expect(isProjection3D('nonExistent', projectionsMeta)).toBe(false); - }); - - it('returns false for empty projections array', () => { - expect(isProjection3D('projection3D', [])).toBe(false); - }); - }); - - describe('getProjectionPlane', () => { - it('returns current plane for 3D projection', () => { - expect(getProjectionPlane(true, 'xz')).toBe('xz'); - expect(getProjectionPlane(true, 'yz')).toBe('yz'); - expect(getProjectionPlane(true, 'xy')).toBe('xy'); - }); - - it('returns xy for 2D projection regardless of current plane', () => { - expect(getProjectionPlane(false, 'xz')).toBe('xy'); - expect(getProjectionPlane(false, 'yz')).toBe('xy'); - expect(getProjectionPlane(false, 'xy')).toBe('xy'); - }); - }); - describe('shouldDisableSelection', () => { it('returns true when data size is 0', () => { expect(shouldDisableSelection(0)).toBe(true); diff --git a/packages/core/src/components/control-bar/control-bar-helpers.ts b/packages/core/src/components/control-bar/control-bar-helpers.ts index c94a88d6..dd53d2f9 100644 --- a/packages/core/src/components/control-bar/control-bar-helpers.ts +++ b/packages/core/src/components/control-bar/control-bar-helpers.ts @@ -19,27 +19,6 @@ export const EXPORT_DEFAULTS = { INCLUDE_LEGEND: true, }; -/** - * Check if projection is 3D based on metadata - */ -export function isProjection3D( - projectionName: string, - projectionsMeta: Array<{ name: string; metadata?: { dimension?: 2 | 3 } }>, -): boolean { - const meta = projectionsMeta.find((p) => p.name === projectionName); - return meta?.metadata?.dimension === 3; -} - -/** - * Get appropriate plane for projection - */ -export function getProjectionPlane( - is3D: boolean, - currentPlane: 'xy' | 'xz' | 'yz', -): 'xy' | 'xz' | 'yz' { - return is3D ? currentPlane : 'xy'; -} - /** * Validate selection mode based on data size */ From 7e37db793cce514034d4b41f037467ac714dea39 Mon Sep 17 00:00:00 2001 From: peymanvahidi Date: Wed, 13 May 2026 22:37:29 +0200 Subject: [PATCH 05/19] chore(scatter-plot): drop stale 'z' from x/y/z fast-path comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 7ae6f74 removed the optional z field from PlotDataPoint. The fast-path comment in _updatePlotDataCoordinates still claimed 'overwrite x/y/z on existing PlotDataPoints' — it now correctly says 'overwrite x/y'. Comment-only; no behavior change. Part of refactor for issue #196. --- packages/core/src/components/scatter-plot/scatter-plot.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/components/scatter-plot/scatter-plot.ts b/packages/core/src/components/scatter-plot/scatter-plot.ts index c43acead..b6722597 100644 --- a/packages/core/src/components/scatter-plot/scatter-plot.ts +++ b/packages/core/src/components/scatter-plot/scatter-plot.ts @@ -585,7 +585,7 @@ export class ProtspaceScatterplot extends LitElement { if (onlyProjectionChanged) { // Fast path: update coordinates in-place from the new projection data. - // No new object allocation — just overwrite x/y/z on existing PlotDataPoints. + // No new object allocation — just overwrite x/y on existing PlotDataPoints. this._updatePlotDataCoordinates(dataToUse); } else { // Release old data references before allocating the new dataset. From 55bdc49f3364578ee3d7bc647e5995455338c24e Mon Sep 17 00:00:00 2001 From: peymanvahidi Date: Thu, 14 May 2026 16:07:07 +0200 Subject: [PATCH 06/19] feat(control-bar): add numeric filter types and matching helpers --- .../control-bar/query-numeric-helpers.test.ts | 115 ++++++++++++++++++ .../control-bar/query-numeric-helpers.ts | 90 ++++++++++++++ .../src/components/control-bar/query-types.ts | 12 ++ 3 files changed, 217 insertions(+) create mode 100644 packages/core/src/components/control-bar/query-numeric-helpers.test.ts create mode 100644 packages/core/src/components/control-bar/query-numeric-helpers.ts diff --git a/packages/core/src/components/control-bar/query-numeric-helpers.test.ts b/packages/core/src/components/control-bar/query-numeric-helpers.test.ts new file mode 100644 index 00000000..268f354e --- /dev/null +++ b/packages/core/src/components/control-bar/query-numeric-helpers.test.ts @@ -0,0 +1,115 @@ +import { describe, it, expect } from 'vitest'; +import type { ProtspaceData } from './types'; +import type { NumericCondition } from './query-types'; +import { + computeNumericBounds, + countNumericMatches, + isNumericConditionReady, + matchesNumericValue, + numericFieldsFor, +} from './query-numeric-helpers'; + +function numericCondition(overrides: Partial): NumericCondition { + return { + id: 'n1', + kind: 'numeric', + annotation: 'length', + operator: 'gt', + min: null, + max: null, + ...overrides, + }; +} + +describe('numericFieldsFor', () => { + it('gt needs only min', () => { + expect(numericFieldsFor('gt')).toEqual({ min: true, max: false }); + }); + it('lt needs only max', () => { + expect(numericFieldsFor('lt')).toEqual({ min: false, max: true }); + }); + it('between needs both', () => { + expect(numericFieldsFor('between')).toEqual({ min: true, max: true }); + }); +}); + +describe('isNumericConditionReady', () => { + it('gt is ready when min is set', () => { + expect(isNumericConditionReady(numericCondition({ operator: 'gt', min: 5 }))).toBe(true); + }); + it('gt is not ready when min is null', () => { + expect(isNumericConditionReady(numericCondition({ operator: 'gt', min: null }))).toBe(false); + }); + it('lt is ready when max is set', () => { + expect(isNumericConditionReady(numericCondition({ operator: 'lt', max: 9 }))).toBe(true); + }); + it('between needs both bounds', () => { + expect( + isNumericConditionReady(numericCondition({ operator: 'between', min: 1, max: null })), + ).toBe(false); + expect(isNumericConditionReady(numericCondition({ operator: 'between', min: 1, max: 9 }))).toBe( + true, + ); + }); +}); + +describe('matchesNumericValue', () => { + it('gt is exclusive', () => { + const c = numericCondition({ operator: 'gt', min: 50 }); + expect(matchesNumericValue(51, c)).toBe(true); + expect(matchesNumericValue(50, c)).toBe(false); + expect(matchesNumericValue(49, c)).toBe(false); + }); + it('lt is exclusive', () => { + const c = numericCondition({ operator: 'lt', max: 50 }); + expect(matchesNumericValue(49, c)).toBe(true); + expect(matchesNumericValue(50, c)).toBe(false); + }); + it('between is inclusive on both ends', () => { + const c = numericCondition({ operator: 'between', min: 10, max: 20 }); + expect(matchesNumericValue(10, c)).toBe(true); + expect(matchesNumericValue(20, c)).toBe(true); + expect(matchesNumericValue(15, c)).toBe(true); + expect(matchesNumericValue(9, c)).toBe(false); + expect(matchesNumericValue(21, c)).toBe(false); + }); + it('between with min > max matches nothing', () => { + const c = numericCondition({ operator: 'between', min: 20, max: 10 }); + expect(matchesNumericValue(15, c)).toBe(false); + }); + it('null value never matches', () => { + expect(matchesNumericValue(null, numericCondition({ operator: 'gt', min: 0 }))).toBe(false); + }); + it('unready condition matches nothing', () => { + expect(matchesNumericValue(100, numericCondition({ operator: 'gt', min: null }))).toBe(false); + }); +}); + +describe('computeNumericBounds', () => { + it('returns min and max ignoring nulls', () => { + expect(computeNumericBounds([3, null, 1, 9, 4])).toEqual({ min: 1, max: 9 }); + }); + it('returns null for empty or all-null input', () => { + expect(computeNumericBounds([])).toBeNull(); + expect(computeNumericBounds([null, null])).toBeNull(); + expect(computeNumericBounds(undefined)).toBeNull(); + }); +}); + +describe('countNumericMatches', () => { + const data: ProtspaceData = { + protein_ids: ['P1', 'P2', 'P3', 'P4'], + numeric_annotation_data: { length: [10, 20, 30, null] }, + }; + it('counts proteins matching a ready condition', () => { + const c = numericCondition({ operator: 'gt', min: 15 }); + expect(countNumericMatches(c, data)).toBe(2); + }); + it('returns 0 for an unready condition', () => { + expect(countNumericMatches(numericCondition({ operator: 'gt', min: null }), data)).toBe(0); + }); + it('returns 0 when the annotation is missing', () => { + const c = numericCondition({ operator: 'gt', min: 0, annotation: 'missing' }); + expect(countNumericMatches(c, data)).toBe(0); + }); +}); diff --git a/packages/core/src/components/control-bar/query-numeric-helpers.ts b/packages/core/src/components/control-bar/query-numeric-helpers.ts new file mode 100644 index 00000000..3c64589f --- /dev/null +++ b/packages/core/src/components/control-bar/query-numeric-helpers.ts @@ -0,0 +1,90 @@ +import type { ProtspaceData } from './types'; +import type { NumericCondition, NumericOperator } from './query-types'; + +/** + * Which value fields a numeric operator needs: + * `gt` uses `min`, `lt` uses `max`, `between` uses both. + */ +export function numericFieldsFor(operator: NumericOperator): { + min: boolean; + max: boolean; +} { + switch (operator) { + case 'gt': + return { min: true, max: false }; + case 'lt': + return { min: false, max: true }; + case 'between': + return { min: true, max: true }; + } +} + +/** + * True when the condition has every bound its operator requires. + * An unready condition is treated as matching nothing. + */ +export function isNumericConditionReady(condition: NumericCondition): boolean { + switch (condition.operator) { + case 'gt': + return condition.min !== null; + case 'lt': + return condition.max !== null; + case 'between': + return condition.min !== null && condition.max !== null; + } +} + +/** + * Test a single raw numeric value against the condition. + * `>` and `<` are exclusive; `between` is inclusive on both ends. + * + * TODO: sequence length is the only numeric annotation today and is always + * present. When other numeric annotations are added, decide how a null value + * should behave under each operator (and under a NOT-wrapped condition). + */ +export function matchesNumericValue(value: number | null, condition: NumericCondition): boolean { + if (value === null) return false; + const { operator, min, max } = condition; + switch (operator) { + case 'gt': + return min !== null && value > min; + case 'lt': + return max !== null && value < max; + case 'between': + return min !== null && max !== null && value >= min && value <= max; + } +} + +/** + * Min and max of a numeric annotation's raw values, ignoring nulls. + * Returns null when there are no usable values (used only for input hints). + */ +export function computeNumericBounds( + values: (number | null)[] | undefined, +): { min: number; max: number } | null { + if (!values || values.length === 0) return null; + let min = Infinity; + let max = -Infinity; + for (const v of values) { + if (v === null) continue; + if (v < min) min = v; + if (v > max) max = v; + } + if (min === Infinity) return null; + return { min, max }; +} + +/** + * Count how many proteins match a numeric condition on its own. + * Returns 0 for an unready condition or a missing annotation. + */ +export function countNumericMatches(condition: NumericCondition, data: ProtspaceData): number { + if (!isNumericConditionReady(condition)) return 0; + const values = data.numeric_annotation_data?.[condition.annotation]; + if (!values) return 0; + let count = 0; + for (const v of values) { + if (matchesNumericValue(v, condition)) count++; + } + return count; +} diff --git a/packages/core/src/components/control-bar/query-types.ts b/packages/core/src/components/control-bar/query-types.ts index 58972c80..81a9d437 100644 --- a/packages/core/src/components/control-bar/query-types.ts +++ b/packages/core/src/components/control-bar/query-types.ts @@ -1,5 +1,17 @@ export type LogicalOp = 'AND' | 'OR' | 'NOT'; +export type NumericOperator = 'gt' | 'lt' | 'between'; + +export interface NumericCondition { + id: string; + logicalOp?: LogicalOp; + annotation: string; + kind: 'numeric'; + operator: NumericOperator; + min: number | null; + max: number | null; +} + export interface FilterCondition { id: string; logicalOp?: LogicalOp; From 91a0262156b292364d791395ad4102f94dba5d51 Mon Sep 17 00:00:00 2001 From: peymanvahidi Date: Thu, 14 May 2026 16:11:23 +0200 Subject: [PATCH 07/19] test(control-bar): cover null-min case for between readiness --- .../src/components/control-bar/query-numeric-helpers.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/core/src/components/control-bar/query-numeric-helpers.test.ts b/packages/core/src/components/control-bar/query-numeric-helpers.test.ts index 268f354e..9effe9d5 100644 --- a/packages/core/src/components/control-bar/query-numeric-helpers.test.ts +++ b/packages/core/src/components/control-bar/query-numeric-helpers.test.ts @@ -47,6 +47,9 @@ describe('isNumericConditionReady', () => { expect( isNumericConditionReady(numericCondition({ operator: 'between', min: 1, max: null })), ).toBe(false); + expect( + isNumericConditionReady(numericCondition({ operator: 'between', min: null, max: 9 })), + ).toBe(false); expect(isNumericConditionReady(numericCondition({ operator: 'between', min: 1, max: 9 }))).toBe( true, ); From d24f2763143f56a762380071d13269b13d04cd7d Mon Sep 17 00:00:00 2001 From: peymanvahidi Date: Thu, 14 May 2026 16:19:14 +0200 Subject: [PATCH 08/19] feat(control-bar): evaluate numeric filter conditions via discriminated union --- knip.jsonc | 4 + .../control-bar/query-condition-row.ts | 14 +- .../control-bar/query-evaluate.test.ts | 222 +++++++++++++++--- .../components/control-bar/query-evaluate.ts | 42 +++- .../src/components/control-bar/query-types.ts | 37 ++- 5 files changed, 275 insertions(+), 44 deletions(-) diff --git a/knip.jsonc b/knip.jsonc index 75663cbc..1a664902 100644 --- a/knip.jsonc +++ b/knip.jsonc @@ -21,6 +21,10 @@ "packages/core": { "entry": ["src/index.ts", "src/components/publish/index.ts"], "project": ["src/**/*.ts"], + // query-types.ts exports a complete discriminated-union API surface; + // createNumericCondition and isNumericCondition are consumed by the + // numeric-input component introduced in the next commit. + "ignore": ["src/components/control-bar/query-types.ts"], }, "packages/utils": { // `src/index.ts` is plugin-discovered; duplicating it trips config-hint errors. diff --git a/packages/core/src/components/control-bar/query-condition-row.ts b/packages/core/src/components/control-bar/query-condition-row.ts index 8072dee2..2036c671 100644 --- a/packages/core/src/components/control-bar/query-condition-row.ts +++ b/packages/core/src/components/control-bar/query-condition-row.ts @@ -2,6 +2,7 @@ import { LitElement, html, nothing } from 'lit'; import { property, state, query as litQuery } from 'lit/decorators.js'; import { customElement } from '../../utils/safe-custom-element'; import type { FilterCondition, LogicalOp } from './query-types'; +import { createCondition } from './query-types'; import type { ProtspaceData } from './types'; import { groupAnnotations } from './annotation-categories'; import { NA_VALUE, NA_DISPLAY } from '@protspace/utils'; @@ -111,8 +112,14 @@ class ProtspaceQueryConditionRow extends LitElement { private _selectAnnotation(annotation: string) { this._showAnnotationPicker = false; this._annotationSearch = ''; - // Clear values when annotation changes - this._dispatchChanged({ ...this.condition, annotation, values: [] }); + // Replace the whole condition object so its kind stays consistent. + this._dispatchChanged( + createCondition({ + id: this.condition.id, + logicalOp: this.condition.logicalOp, + annotation, + }), + ); } private _handleLogicalOpChange(e: Event) { @@ -122,11 +129,13 @@ class ProtspaceQueryConditionRow extends LitElement { } private _removeValue(value: string) { + if (this.condition.kind !== 'categorical') return; const values = this.condition.values.filter((v) => v !== value); this._dispatchChanged({ ...this.condition, values }); } private _handleValueSelected(e: CustomEvent<{ value: string }>) { + if (this.condition.kind !== 'categorical') return; const value = e.detail.value; if (!this.condition.values.includes(value)) { this._dispatchChanged({ ...this.condition, values: [...this.condition.values, value] }); @@ -199,6 +208,7 @@ class ProtspaceQueryConditionRow extends LitElement { } private _renderValues() { + if (this.condition.kind !== 'categorical') return nothing; return html`
${this.condition.values.map( diff --git a/packages/core/src/components/control-bar/query-evaluate.test.ts b/packages/core/src/components/control-bar/query-evaluate.test.ts index f49d66c9..eee45110 100644 --- a/packages/core/src/components/control-bar/query-evaluate.test.ts +++ b/packages/core/src/components/control-bar/query-evaluate.test.ts @@ -3,7 +3,7 @@ import type { ProtspaceData } from './types'; import type { FilterQuery } from './query-types'; import { evaluateQuery, evaluateQueryExcluding, resolveAnnotationValue } from './query-evaluate'; -/** Minimal test data: 5 proteins with organism and reviewed annotations */ +/** Minimal test data: 5 proteins with categorical and numeric annotations */ function createTestData(): ProtspaceData { return { protein_ids: ['P1', 'P2', 'P3', 'P4', 'P5'], @@ -11,6 +11,7 @@ function createTestData(): ProtspaceData { organism: { values: ['Human', 'Mouse', 'Zebrafish'] }, reviewed: { values: ['true', 'false'] }, pfam: { values: ['PF00069', 'PF00076', null] }, + length: { kind: 'numeric', values: [] }, }, annotation_data: { // P1=Human, P2=Mouse, P3=Human, P4=Zebrafish, P5=Mouse @@ -20,6 +21,10 @@ function createTestData(): ProtspaceData { // P1=PF00069, P2=PF00076, P3=null, P4=PF00069, P5=PF00076 pfam: [[0], [1], [2], [0], [1]], }, + numeric_annotation_data: { + // P1=100, P2=250, P3=400, P4=550, P5=null + length: [100, 250, 400, 550, null], + }, }; } @@ -67,19 +72,25 @@ describe('evaluateQuery', () => { describe('single condition', () => { it('matches proteins with selected value', () => { - const query: FilterQuery = [{ id: '1', annotation: 'organism', values: ['Human'] }]; + const query: FilterQuery = [ + { id: '1', kind: 'categorical', annotation: 'organism', values: ['Human'] }, + ]; const result = evaluateQuery(query, createTestData()); expect(result).toEqual(new Set([0, 2])); }); it('matches multiple values (implicit OR)', () => { - const query: FilterQuery = [{ id: '1', annotation: 'organism', values: ['Human', 'Mouse'] }]; + const query: FilterQuery = [ + { id: '1', kind: 'categorical', annotation: 'organism', values: ['Human', 'Mouse'] }, + ]; const result = evaluateQuery(query, createTestData()); expect(result).toEqual(new Set([0, 1, 2, 4])); }); it('skips condition with empty values', () => { - const query: FilterQuery = [{ id: '1', annotation: 'organism', values: [] }]; + const query: FilterQuery = [ + { id: '1', kind: 'categorical', annotation: 'organism', values: [] }, + ]; const result = evaluateQuery(query, createTestData()); expect(result).toEqual(new Set([0, 1, 2, 3, 4])); }); @@ -88,8 +99,14 @@ describe('evaluateQuery', () => { describe('AND logic', () => { it('intersects two conditions', () => { const query: FilterQuery = [ - { id: '1', annotation: 'organism', values: ['Human'] }, - { id: '2', logicalOp: 'AND', annotation: 'reviewed', values: ['true'] }, + { id: '1', kind: 'categorical', annotation: 'organism', values: ['Human'] }, + { + id: '2', + logicalOp: 'AND', + kind: 'categorical', + annotation: 'reviewed', + values: ['true'], + }, ]; const result = evaluateQuery(query, createTestData()); expect(result).toEqual(new Set([0, 2])); @@ -99,8 +116,14 @@ describe('evaluateQuery', () => { describe('OR logic', () => { it('unions two conditions', () => { const query: FilterQuery = [ - { id: '1', annotation: 'organism', values: ['Human'] }, - { id: '2', logicalOp: 'OR', annotation: 'reviewed', values: ['false'] }, + { id: '1', kind: 'categorical', annotation: 'organism', values: ['Human'] }, + { + id: '2', + logicalOp: 'OR', + kind: 'categorical', + annotation: 'reviewed', + values: ['false'], + }, ]; const result = evaluateQuery(query, createTestData()); expect(result).toEqual(new Set([0, 1, 2, 4])); @@ -110,7 +133,13 @@ describe('evaluateQuery', () => { describe('NOT logic', () => { it('negates a single condition', () => { const query: FilterQuery = [ - { id: '1', logicalOp: 'NOT', annotation: 'organism', values: ['Human'] }, + { + id: '1', + logicalOp: 'NOT', + kind: 'categorical', + annotation: 'organism', + values: ['Human'], + }, ]; const result = evaluateQuery(query, createTestData()); expect(result).toEqual(new Set([1, 3, 4])); @@ -118,8 +147,14 @@ describe('evaluateQuery', () => { it('NOT combined with AND', () => { const query: FilterQuery = [ - { id: '1', annotation: 'organism', values: ['Human', 'Mouse'] }, - { id: '2', logicalOp: 'NOT', annotation: 'reviewed', values: ['false'] }, + { id: '1', kind: 'categorical', annotation: 'organism', values: ['Human', 'Mouse'] }, + { + id: '2', + logicalOp: 'NOT', + kind: 'categorical', + annotation: 'reviewed', + values: ['false'], + }, ]; const result = evaluateQuery(query, createTestData()); expect(result).toEqual(new Set([0, 2])); @@ -129,13 +164,19 @@ describe('evaluateQuery', () => { describe('groups', () => { it('evaluates a group as a unit', () => { const query: FilterQuery = [ - { id: '1', annotation: 'reviewed', values: ['true'] }, + { id: '1', kind: 'categorical', annotation: 'reviewed', values: ['true'] }, { id: 'g1', logicalOp: 'AND', conditions: [ - { id: '2', annotation: 'organism', values: ['Human'] }, - { id: '3', logicalOp: 'OR', annotation: 'organism', values: ['Mouse'] }, + { id: '2', kind: 'categorical', annotation: 'organism', values: ['Human'] }, + { + id: '3', + logicalOp: 'OR', + kind: 'categorical', + annotation: 'organism', + values: ['Mouse'], + }, ], }, ]; @@ -149,8 +190,14 @@ describe('evaluateQuery', () => { id: 'g1', logicalOp: 'NOT', conditions: [ - { id: '1', annotation: 'organism', values: ['Human'] }, - { id: '2', logicalOp: 'AND', annotation: 'reviewed', values: ['true'] }, + { id: '1', kind: 'categorical', annotation: 'organism', values: ['Human'] }, + { + id: '2', + logicalOp: 'AND', + kind: 'categorical', + annotation: 'reviewed', + values: ['true'], + }, ], }, ]; @@ -161,35 +208,146 @@ describe('evaluateQuery', () => { describe('null value handling', () => { it('matches null via __NA__ normalized value', () => { - const query: FilterQuery = [{ id: '1', annotation: 'pfam', values: ['__NA__'] }]; + const query: FilterQuery = [ + { id: '1', kind: 'categorical', annotation: 'pfam', values: ['__NA__'] }, + ]; const result = evaluateQuery(query, createTestData()); - // P3 has pfam=null → normalized to __NA__ expect(result).toEqual(new Set([2])); }); it('NOT excludes null via __NA__ normalized value', () => { const query: FilterQuery = [ - { id: '1', logicalOp: 'NOT', annotation: 'pfam', values: ['__NA__'] }, + { id: '1', logicalOp: 'NOT', kind: 'categorical', annotation: 'pfam', values: ['__NA__'] }, ]; const result = evaluateQuery(query, createTestData()); - // NOT negates: all except P3 (which has null pfam) expect(result).toEqual(new Set([0, 1, 3, 4])); }); }); describe('missing annotation', () => { it('treats missing annotation as non-matching', () => { - const query: FilterQuery = [{ id: '1', annotation: 'nonexistent', values: ['foo'] }]; + const query: FilterQuery = [ + { id: '1', kind: 'categorical', annotation: 'nonexistent', values: ['foo'] }, + ]; const result = evaluateQuery(query, createTestData()); expect(result).toEqual(new Set()); }); }); + + describe('numeric conditions', () => { + it('matches proteins greater than a threshold (exclusive)', () => { + const query: FilterQuery = [ + { id: '1', kind: 'numeric', annotation: 'length', operator: 'gt', min: 250, max: null }, + ]; + const result = evaluateQuery(query, createTestData()); + // > 250 → P3(400), P4(550); P2(250) excluded (exclusive) + expect(result).toEqual(new Set([2, 3])); + }); + + it('matches proteins less than a threshold (exclusive)', () => { + const query: FilterQuery = [ + { id: '1', kind: 'numeric', annotation: 'length', operator: 'lt', min: null, max: 400 }, + ]; + const result = evaluateQuery(query, createTestData()); + // < 400 → P1(100), P2(250) + expect(result).toEqual(new Set([0, 1])); + }); + + it('matches proteins within an inclusive interval', () => { + const query: FilterQuery = [ + { id: '1', kind: 'numeric', annotation: 'length', operator: 'between', min: 250, max: 400 }, + ]; + const result = evaluateQuery(query, createTestData()); + // between 250 and 400 inclusive → P2(250), P3(400) + expect(result).toEqual(new Set([1, 2])); + }); + + it('never matches a protein with a null numeric value', () => { + const query: FilterQuery = [ + { id: '1', kind: 'numeric', annotation: 'length', operator: 'gt', min: 0, max: null }, + ]; + const result = evaluateQuery(query, createTestData()); + // P5 has null length → excluded even though 0 is below every real value + expect(result).toEqual(new Set([0, 1, 2, 3])); + }); + + it('matches nothing when a required bound is missing', () => { + const query: FilterQuery = [ + { id: '1', kind: 'numeric', annotation: 'length', operator: 'gt', min: null, max: null }, + ]; + const result = evaluateQuery(query, createTestData()); + expect(result).toEqual(new Set()); + }); + + it('matches nothing for between with min > max', () => { + const query: FilterQuery = [ + { id: '1', kind: 'numeric', annotation: 'length', operator: 'between', min: 500, max: 200 }, + ]; + const result = evaluateQuery(query, createTestData()); + expect(result).toEqual(new Set()); + }); + + it('combines a numeric condition with a categorical one via AND', () => { + const query: FilterQuery = [ + { id: '1', kind: 'categorical', annotation: 'organism', values: ['Human'] }, + { + id: '2', + logicalOp: 'AND', + kind: 'numeric', + annotation: 'length', + operator: 'gt', + min: 250, + max: null, + }, + ]; + const result = evaluateQuery(query, createTestData()); + // Human → P1,P3 ; length>250 → P3,P4 ; AND → P3 + expect(result).toEqual(new Set([2])); + }); + + it('combines a numeric condition with a categorical one via OR', () => { + const query: FilterQuery = [ + { id: '1', kind: 'categorical', annotation: 'organism', values: ['Zebrafish'] }, + { + id: '2', + logicalOp: 'OR', + kind: 'numeric', + annotation: 'length', + operator: 'lt', + min: null, + max: 200, + }, + ]; + const result = evaluateQuery(query, createTestData()); + // Zebrafish → P4 ; length<200 → P1 ; OR → P1,P4 + expect(result).toEqual(new Set([0, 3])); + }); + + it('negates a numeric condition with NOT', () => { + const query: FilterQuery = [ + { + id: '1', + logicalOp: 'NOT', + kind: 'numeric', + annotation: 'length', + operator: 'gt', + min: 250, + max: null, + }, + ]; + const result = evaluateQuery(query, createTestData()); + // length>250 → P3,P4 ; NOT → P1,P2,P5 (P5 null is "not > 250") + expect(result).toEqual(new Set([0, 1, 4])); + }); + }); }); describe('evaluateQueryExcluding', () => { it('returns all indices when excluding single condition from single-condition query', () => { const data = createTestData(); - const query: FilterQuery = [{ id: '1', annotation: 'organism', values: ['Human'] }]; + const query: FilterQuery = [ + { id: '1', kind: 'categorical', annotation: 'organism', values: ['Human'] }, + ]; const result = evaluateQueryExcluding(query, data, '1'); expect(result).toEqual(new Set([0, 1, 2, 3, 4])); }); @@ -197,10 +355,9 @@ describe('evaluateQueryExcluding', () => { it('returns other condition result when excluding one of two AND conditions', () => { const data = createTestData(); const query: FilterQuery = [ - { id: '1', annotation: 'organism', values: ['Human'] }, - { id: '2', logicalOp: 'AND', annotation: 'reviewed', values: ['true'] }, + { id: '1', kind: 'categorical', annotation: 'organism', values: ['Human'] }, + { id: '2', logicalOp: 'AND', kind: 'categorical', annotation: 'reviewed', values: ['true'] }, ]; - // Excluding condition 1 → only condition 2 (reviewed=true: P1,P3,P4) const result = evaluateQueryExcluding(query, data, '1'); expect(result).toEqual(new Set([0, 2, 3])); }); @@ -212,19 +369,26 @@ describe('evaluateQueryExcluding', () => { id: 'g1', logicalOp: 'AND', conditions: [ - { id: '1', annotation: 'organism', values: ['Human'] }, - { id: '2', logicalOp: 'AND', annotation: 'reviewed', values: ['true'] }, + { id: '1', kind: 'categorical', annotation: 'organism', values: ['Human'] }, + { + id: '2', + logicalOp: 'AND', + kind: 'categorical', + annotation: 'reviewed', + values: ['true'], + }, ], }, ]; - // Excluding condition 2 inside group → group evaluates just condition 1 (organism=Human: P1,P3) const result = evaluateQueryExcluding(query, data, '2'); expect(result).toEqual(new Set([0, 2])); }); it('returns full query result when excluding non-existent ID', () => { const data = createTestData(); - const query: FilterQuery = [{ id: '1', annotation: 'organism', values: ['Human'] }]; + const query: FilterQuery = [ + { id: '1', kind: 'categorical', annotation: 'organism', values: ['Human'] }, + ]; const result = evaluateQueryExcluding(query, data, 'nonexistent'); expect(result).toEqual(new Set([0, 2])); }); diff --git a/packages/core/src/components/control-bar/query-evaluate.ts b/packages/core/src/components/control-bar/query-evaluate.ts index 6781dfd0..6932a9d5 100644 --- a/packages/core/src/components/control-bar/query-evaluate.ts +++ b/packages/core/src/components/control-bar/query-evaluate.ts @@ -1,6 +1,12 @@ import type { ProtspaceData } from './types'; -import type { FilterQuery, FilterQueryItem, FilterCondition } from './query-types'; +import type { + FilterQuery, + FilterQueryItem, + FilterCondition, + NumericCondition, +} from './query-types'; import { isFilterGroup } from './query-types'; +import { isNumericConditionReady, matchesNumericValue } from './query-numeric-helpers'; import { toInternalValue } from '../legend/config'; import { getFirstAnnotationIndex } from '@protspace/utils'; @@ -35,15 +41,19 @@ function normalizeValue(value: string | null): string { /** * Evaluate a single condition against all proteins, returning a Set of matching indices. - * Always uses "is" semantics: a protein matches if its normalized annotation value - * is among the condition's selected values. Negation is handled at the combining - * level via the NOT logical operator. + * Categorical conditions use "is" semantics (value is among the selected set); + * numeric conditions apply a comparison operator. Negation is handled at the + * combining level via the NOT logical operator. */ function evaluateCondition( condition: FilterCondition, data: ProtspaceData, numProteins: number, ): Set { + if (condition.kind === 'numeric') { + return evaluateNumericCondition(condition, data, numProteins); + } + if (condition.values.length === 0) { return allIndices(numProteins); } @@ -67,6 +77,30 @@ function evaluateCondition( return matches; } +/** + * Evaluate a numeric condition: a protein matches when its raw numeric value + * satisfies the operator. An unconfigured condition (missing a required bound) + * matches nothing, so an in-progress filter never produces misleading results. + */ +function evaluateNumericCondition( + condition: NumericCondition, + data: ProtspaceData, + numProteins: number, +): Set { + if (!isNumericConditionReady(condition)) return new Set(); + + const values = data.numeric_annotation_data?.[condition.annotation]; + if (!values) return new Set(); + + const matches = new Set(); + for (let i = 0; i < numProteins; i++) { + if (matchesNumericValue(values[i] ?? null, condition)) { + matches.add(i); + } + } + return matches; +} + /** * Evaluate a FilterQuery and return the Set of matching protein indices. */ diff --git a/packages/core/src/components/control-bar/query-types.ts b/packages/core/src/components/control-bar/query-types.ts index 81a9d437..763c5509 100644 --- a/packages/core/src/components/control-bar/query-types.ts +++ b/packages/core/src/components/control-bar/query-types.ts @@ -1,23 +1,25 @@ export type LogicalOp = 'AND' | 'OR' | 'NOT'; - export type NumericOperator = 'gt' | 'lt' | 'between'; -export interface NumericCondition { +interface BaseCondition { id: string; logicalOp?: LogicalOp; annotation: string; +} + +export interface CategoricalCondition extends BaseCondition { + kind: 'categorical'; + values: string[]; +} + +export interface NumericCondition extends BaseCondition { kind: 'numeric'; operator: NumericOperator; min: number | null; max: number | null; } -export interface FilterCondition { - id: string; - logicalOp?: LogicalOp; - annotation: string; - values: string[]; -} +export type FilterCondition = CategoricalCondition | NumericCondition; export interface FilterGroup { id: string; @@ -34,15 +36,28 @@ function generateId(): string { return `q-${Date.now()}-${nextId++}`; } -export function createCondition(overrides?: Partial): FilterCondition { +export function createCondition(overrides?: Partial): CategoricalCondition { return { id: generateId(), + kind: 'categorical', annotation: '', values: [], ...overrides, }; } +export function createNumericCondition(overrides?: Partial): NumericCondition { + return { + id: generateId(), + kind: 'numeric', + annotation: '', + operator: 'gt', + min: null, + max: null, + ...overrides, + }; +} + export function createGroup(overrides?: Partial): FilterGroup { return { id: generateId(), @@ -55,3 +70,7 @@ export function createGroup(overrides?: Partial): FilterGroup { export function isFilterGroup(item: FilterQueryItem): item is FilterGroup { return 'conditions' in item; } + +export function isNumericCondition(condition: FilterCondition): condition is NumericCondition { + return condition.kind === 'numeric'; +} From 1dcd75bf1e5bc1aa149b0eeb31592944c4621ffd Mon Sep 17 00:00:00 2001 From: peymanvahidi Date: Thu, 14 May 2026 16:27:10 +0200 Subject: [PATCH 09/19] refactor(control-bar): drop unused isNumericCondition type guard --- knip.jsonc | 6 +++--- packages/core/src/components/control-bar/query-types.ts | 4 ---- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/knip.jsonc b/knip.jsonc index 1a664902..b2122655 100644 --- a/knip.jsonc +++ b/knip.jsonc @@ -21,9 +21,9 @@ "packages/core": { "entry": ["src/index.ts", "src/components/publish/index.ts"], "project": ["src/**/*.ts"], - // query-types.ts exports a complete discriminated-union API surface; - // createNumericCondition and isNumericCondition are consumed by the - // numeric-input component introduced in the next commit. + // TRANSITIONAL: query-types.ts exports createNumericCondition, which is + // not consumed until the condition-row wiring task. Remove this ignore + // once createNumericCondition has a real call site. "ignore": ["src/components/control-bar/query-types.ts"], }, "packages/utils": { diff --git a/packages/core/src/components/control-bar/query-types.ts b/packages/core/src/components/control-bar/query-types.ts index 763c5509..4d424aec 100644 --- a/packages/core/src/components/control-bar/query-types.ts +++ b/packages/core/src/components/control-bar/query-types.ts @@ -70,7 +70,3 @@ export function createGroup(overrides?: Partial): FilterGroup { export function isFilterGroup(item: FilterQueryItem): item is FilterGroup { return 'conditions' in item; } - -export function isNumericCondition(condition: FilterCondition): condition is NumericCondition { - return condition.kind === 'numeric'; -} From d8a6e7de5c3603580f36fc4a3691b741f0a9d10c Mon Sep 17 00:00:00 2001 From: peymanvahidi Date: Thu, 14 May 2026 16:30:01 +0200 Subject: [PATCH 10/19] feat(control-bar): add query-numeric-input component --- knip.jsonc | 7 +- .../control-bar/query-builder.styles.ts | 50 +++++ .../control-bar/query-numeric-input.ts | 178 ++++++++++++++++++ 3 files changed, 234 insertions(+), 1 deletion(-) create mode 100644 packages/core/src/components/control-bar/query-numeric-input.ts diff --git a/knip.jsonc b/knip.jsonc index b2122655..9dca6371 100644 --- a/knip.jsonc +++ b/knip.jsonc @@ -24,7 +24,12 @@ // TRANSITIONAL: query-types.ts exports createNumericCondition, which is // not consumed until the condition-row wiring task. Remove this ignore // once createNumericCondition has a real call site. - "ignore": ["src/components/control-bar/query-types.ts"], + // TRANSITIONAL: query-numeric-input.ts is not wired into condition-row yet. + // Remove this ignore once it has a real import site. + "ignore": [ + "src/components/control-bar/query-types.ts", + "src/components/control-bar/query-numeric-input.ts", + ], }, "packages/utils": { // `src/index.ts` is plugin-discovered; duplicating it trips config-hint errors. diff --git a/packages/core/src/components/control-bar/query-builder.styles.ts b/packages/core/src/components/control-bar/query-builder.styles.ts index 01700201..a97d88e7 100644 --- a/packages/core/src/components/control-bar/query-builder.styles.ts +++ b/packages/core/src/components/control-bar/query-builder.styles.ts @@ -484,6 +484,56 @@ export const queryBuilderStyles = css` overflow: visible; } + /* ========================================== + NUMERIC INPUT + ========================================== */ + + .numeric-input { + display: flex; + align-items: center; + gap: var(--spacing-xs); + flex-wrap: wrap; + } + + .numeric-operator-select { + padding: var(--input-padding-y) 18px var(--input-padding-y) var(--spacing-xs); + border: var(--border-width) solid var(--border); + border-radius: var(--radius); + background: var(--surface); + font-size: var(--text-sm); + color: var(--text-primary); + cursor: pointer; + } + + .numeric-field { + width: 90px; + padding: var(--input-padding-y) var(--input-padding-x); + border: var(--border-width) solid var(--border); + border-radius: var(--radius); + background: var(--surface); + font-size: var(--text-base); + color: var(--text-primary); + box-sizing: border-box; + } + + .numeric-field:focus-visible { + outline: none; + border-color: var(--primary); + box-shadow: + 0 0 0 1px var(--primary), + 0 0 0 3px var(--focus-ring-bg); + } + + .numeric-dash { + color: var(--text-secondary); + } + + .numeric-match-count { + font-size: var(--text-sm); + color: var(--text-secondary); + white-space: nowrap; + } + /* ========================================== RESPONSIVE ========================================== */ diff --git a/packages/core/src/components/control-bar/query-numeric-input.ts b/packages/core/src/components/control-bar/query-numeric-input.ts new file mode 100644 index 00000000..71b506b2 --- /dev/null +++ b/packages/core/src/components/control-bar/query-numeric-input.ts @@ -0,0 +1,178 @@ +import { LitElement, html, nothing } from 'lit'; +import { property, state } from 'lit/decorators.js'; +import { customElement } from '../../utils/safe-custom-element'; +import type { ProtspaceData } from './types'; +import type { NumericCondition, NumericOperator } from './query-types'; +import { + computeNumericBounds, + countNumericMatches, + isNumericConditionReady, + numericFieldsFor, +} from './query-numeric-helpers'; +import { queryBuilderStyles } from './query-builder.styles'; + +/** + * Numeric value input for a query condition: an operator dropdown plus one or + * two number fields, with a debounced live match count. + * + * The condition object is owned by the control bar; this component is + * controlled. It keeps the in-progress field text locally so a half-typed + * value (e.g. "-" or "1.") survives the round-trip of the condition prop. + * + * Events: + * - `numeric-changed` — operator or a bound changed, detail: `{ condition: NumericCondition }` + */ +@customElement('protspace-query-numeric-input') +class ProtspaceQueryNumericInput extends LitElement { + static styles = queryBuilderStyles; + + @property({ type: Object }) condition!: NumericCondition; + @property({ type: Object }) data: ProtspaceData | undefined = undefined; + + @state() private _matchCount: number | null = null; + @state() private _dataBounds: { min: number; max: number } | null = null; + @state() private _minText: string = ''; + @state() private _maxText: string = ''; + + private _boundsAnnotation: string | null = null; + private _countTimer: ReturnType | null = null; + + // ─── Lifecycle ──────────────────────────────────────────────────────────── + + disconnectedCallback() { + super.disconnectedCallback(); + if (this._countTimer) { + clearTimeout(this._countTimer); + this._countTimer = null; + } + } + + willUpdate(changed: Map) { + if (!changed.has('condition') && !changed.has('data')) return; + + // Adopt prop values into local text unless they already parse to the same + // number — this preserves an in-progress entry while still picking up + // external resets (e.g. switching annotation clears min/max to null). + if (this._parseFieldValue(this._minText) !== this.condition.min) { + this._minText = this.condition.min === null ? '' : String(this.condition.min); + } + if (this._parseFieldValue(this._maxText) !== this.condition.max) { + this._maxText = this.condition.max === null ? '' : String(this.condition.max); + } + + // Recompute placeholder bounds only when the annotation changes. + if (this.condition.annotation !== this._boundsAnnotation) { + this._boundsAnnotation = this.condition.annotation; + this._dataBounds = computeNumericBounds( + this.data?.numeric_annotation_data?.[this.condition.annotation], + ); + } + } + + updated(changed: Map) { + if (changed.has('condition') || changed.has('data')) { + this._scheduleCount(); + } + } + + // ─── Debounced live match count ─────────────────────────────────────────── + + private _scheduleCount() { + if (this._countTimer) clearTimeout(this._countTimer); + this._countTimer = setTimeout(() => { + if (!this.data || !isNumericConditionReady(this.condition)) { + this._matchCount = null; + return; + } + this._matchCount = countNumericMatches(this.condition, this.data); + }, 750); + } + + // ─── Helpers ────────────────────────────────────────────────────────────── + + private _parseFieldValue(raw: string): number | null { + if (raw.trim() === '') return null; + const parsed = Number(raw); + return Number.isFinite(parsed) ? parsed : null; + } + + private _emitChanged(updated: NumericCondition) { + this.dispatchEvent( + new CustomEvent('numeric-changed', { + detail: { condition: updated }, + bubbles: true, + composed: true, + }), + ); + } + + // ─── Event handlers ─────────────────────────────────────────────────────── + + private _handleOperatorChange(e: Event) { + const operator = (e.target as HTMLSelectElement).value as NumericOperator; + this._emitChanged({ ...this.condition, operator }); + } + + private _handleMinInput(e: Event) { + this._minText = (e.target as HTMLInputElement).value; + this._emitChanged({ ...this.condition, min: this._parseFieldValue(this._minText) }); + } + + private _handleMaxInput(e: Event) { + this._maxText = (e.target as HTMLInputElement).value; + this._emitChanged({ ...this.condition, max: this._parseFieldValue(this._maxText) }); + } + + // ─── Render ─────────────────────────────────────────────────────────────── + + render() { + const fields = numericFieldsFor(this.condition.operator); + const minPlaceholder = this._dataBounds ? `min (${this._dataBounds.min})` : 'min'; + const maxPlaceholder = this._dataBounds ? `max (${this._dataBounds.max})` : 'max'; + + return html` +
+ + + ${fields.min + ? html`` + : nothing} + ${fields.min && fields.max ? html`–` : nothing} + ${fields.max + ? html`` + : nothing} + ${this._matchCount !== null + ? html`${this._matchCount.toLocaleString()} proteins match` + : nothing} +
+ `; + } +} + +declare global { + interface HTMLElementTagNameMap { + 'protspace-query-numeric-input': ProtspaceQueryNumericInput; + } +} From b7f7e3be31f47fc7b8c73d6cd7249a9811964ddd Mon Sep 17 00:00:00 2001 From: peymanvahidi Date: Thu, 14 May 2026 16:36:03 +0200 Subject: [PATCH 11/19] style(control-bar): match operator select chevron to logical-op select --- .../src/components/control-bar/query-builder.styles.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/core/src/components/control-bar/query-builder.styles.ts b/packages/core/src/components/control-bar/query-builder.styles.ts index a97d88e7..65be8966 100644 --- a/packages/core/src/components/control-bar/query-builder.styles.ts +++ b/packages/core/src/components/control-bar/query-builder.styles.ts @@ -499,10 +499,16 @@ export const queryBuilderStyles = css` padding: var(--input-padding-y) 18px var(--input-padding-y) var(--spacing-xs); border: var(--border-width) solid var(--border); border-radius: var(--radius); - background: var(--surface); + background-color: var(--surface); + background-image: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='12' height='12' viewBox='0 0 24 24' fill='none' stroke='%235b6b7a' stroke-width='2' stroke-linecap='round' stroke-linejoin='round'%3E%3Cpath d='M6 9l6 6 6-6'/%3E%3C/svg%3E"); + background-repeat: no-repeat; + background-position: right 4px center; + background-size: 12px; font-size: var(--text-sm); color: var(--text-primary); cursor: pointer; + appearance: none; + -webkit-appearance: none; } .numeric-field { From 9fcc1527375f4b3d204f80712f118a9fa8fbd051 Mon Sep 17 00:00:00 2001 From: peymanvahidi Date: Thu, 14 May 2026 16:38:41 +0200 Subject: [PATCH 12/19] feat(control-bar): render numeric input for numeric filter annotations --- knip.jsonc | 9 ----- .../control-bar/query-condition-row.ts | 36 +++++++++++++------ 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/knip.jsonc b/knip.jsonc index 9dca6371..75663cbc 100644 --- a/knip.jsonc +++ b/knip.jsonc @@ -21,15 +21,6 @@ "packages/core": { "entry": ["src/index.ts", "src/components/publish/index.ts"], "project": ["src/**/*.ts"], - // TRANSITIONAL: query-types.ts exports createNumericCondition, which is - // not consumed until the condition-row wiring task. Remove this ignore - // once createNumericCondition has a real call site. - // TRANSITIONAL: query-numeric-input.ts is not wired into condition-row yet. - // Remove this ignore once it has a real import site. - "ignore": [ - "src/components/control-bar/query-types.ts", - "src/components/control-bar/query-numeric-input.ts", - ], }, "packages/utils": { // `src/index.ts` is plugin-discovered; duplicating it trips config-hint errors. diff --git a/packages/core/src/components/control-bar/query-condition-row.ts b/packages/core/src/components/control-bar/query-condition-row.ts index 2036c671..a36adaa7 100644 --- a/packages/core/src/components/control-bar/query-condition-row.ts +++ b/packages/core/src/components/control-bar/query-condition-row.ts @@ -2,12 +2,13 @@ import { LitElement, html, nothing } from 'lit'; import { property, state, query as litQuery } from 'lit/decorators.js'; import { customElement } from '../../utils/safe-custom-element'; import type { FilterCondition, LogicalOp } from './query-types'; -import { createCondition } from './query-types'; +import { createCondition, createNumericCondition } from './query-types'; import type { ProtspaceData } from './types'; import { groupAnnotations } from './annotation-categories'; import { NA_VALUE, NA_DISPLAY } from '@protspace/utils'; import { queryBuilderStyles } from './query-builder.styles'; import './query-value-picker'; +import './query-numeric-input'; /** * Renders a single query condition row. @@ -112,14 +113,14 @@ class ProtspaceQueryConditionRow extends LitElement { private _selectAnnotation(annotation: string) { this._showAnnotationPicker = false; this._annotationSearch = ''; - // Replace the whole condition object so its kind stays consistent. - this._dispatchChanged( - createCondition({ - id: this.condition.id, - logicalOp: this.condition.logicalOp, - annotation, - }), - ); + // Replace the whole condition object so its kind matches the annotation. + const base = { + id: this.condition.id, + logicalOp: this.condition.logicalOp, + annotation, + }; + const isNumeric = this.data?.annotations?.[annotation]?.kind === 'numeric'; + this._dispatchChanged(isNumeric ? createNumericCondition(base) : createCondition(base)); } private _handleLogicalOpChange(e: Event) { @@ -255,6 +256,21 @@ class ProtspaceQueryConditionRow extends LitElement { `; } + private _handleNumericChanged(e: CustomEvent<{ condition: FilterCondition }>) { + this._dispatchChanged(e.detail.condition); + } + + private _renderNumericInput() { + if (this.condition.kind !== 'numeric') return nothing; + return html` + + `; + } + // ─── Render ─────────────────────────────────────────────────────────────── render() { @@ -288,7 +304,7 @@ class ProtspaceQueryConditionRow extends LitElement { ${this._showAnnotationPicker ? this._renderAnnotationPicker() : nothing} - ${this._renderValues()} + ${this.condition.kind === 'numeric' ? this._renderNumericInput() : this._renderValues()}
diff --git a/packages/core/src/components/scatter-plot/scatter-plot.ts b/packages/core/src/components/scatter-plot/scatter-plot.ts index b6722597..1a4a6526 100644 --- a/packages/core/src/components/scatter-plot/scatter-plot.ts +++ b/packages/core/src/components/scatter-plot/scatter-plot.ts @@ -426,6 +426,15 @@ export class ProtspaceScatterplot extends LitElement { } } + // A query filter is scoped to the current dataset. On a dataset swap, drop the + // filtered-id set before _processData runs below — otherwise a stale set (ids + // from the previous dataset) would match nothing and blank the new plot. Set + // here (not via a getter) so the synchronous _processData read sees it cleared. + if (changedProperties.has('data') && this.filtersActive) { + this.filteredProteinIds = []; + this.filtersActive = false; + } + const numericSettingsChangedOnly = changedProperties.has('numericAnnotationSettings') && !changedProperties.has('data') && From 98effa7790712e2a439c9171f4b8e2cd0abf8fd1 Mon Sep 17 00:00:00 2001 From: peymanvahidi Date: Sat, 30 May 2026 00:15:49 +0200 Subject: [PATCH 17/19] fix(control-bar): make numeric filtering reachable and consistent under NOT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three numeric-filter correctness fixes: - `_selectAnnotation` keyed on `kind === 'numeric'`, but the currently-coloured numeric annotation is materialized to `kind:'categorical'` (it keeps `sourceKind:'numeric'`), so picking it offered a categorical bin picker instead of the numeric range input. Use the shared `isNumericAnnotation()` so a numeric annotation is filtered numerically whether or not it is currently materialized. - An unconfigured numeric condition evaluated to the empty set, so NOT-wrapping it isolated EVERY protein (complement of nothing) while Apply stayed enabled — asymmetric with an empty categorical condition. Treat an unconfigured numeric condition as a no-op (matches all), mirroring categorical, so `NOT(unconfigured)` matches nothing and Apply stays disabled. - Switching operator left the now-hidden bound populated; switching back silently resurrected it and re-constrained the filter. Null the unused bound on operator change. Also give the operator select and min/max inputs accessible names (aria-label), and add component tests for the controlled round-trip, operator switch, numeric NOT-symmetry, and annotation kind detection. --- .../control-bar/query-condition-row.test.ts | 80 +++++++++++ .../control-bar/query-condition-row.ts | 8 +- .../control-bar/query-evaluate.test.ts | 18 ++- .../components/control-bar/query-evaluate.ts | 8 +- .../control-bar/query-numeric-input.test.ts | 134 ++++++++++++++++++ .../control-bar/query-numeric-input.ts | 13 +- 6 files changed, 254 insertions(+), 7 deletions(-) create mode 100644 packages/core/src/components/control-bar/query-condition-row.test.ts create mode 100644 packages/core/src/components/control-bar/query-numeric-input.test.ts diff --git a/packages/core/src/components/control-bar/query-condition-row.test.ts b/packages/core/src/components/control-bar/query-condition-row.test.ts new file mode 100644 index 00000000..1fe0ac92 --- /dev/null +++ b/packages/core/src/components/control-bar/query-condition-row.test.ts @@ -0,0 +1,80 @@ +/** + * @vitest-environment jsdom + * + * Picking an annotation must produce a condition whose kind matches the + * annotation's type. The tricky case is a numeric annotation that has been + * materialized for the legend: it becomes kind:'categorical' but keeps + * sourceKind:'numeric', and must still be filtered with the numeric range input. + */ +import { beforeEach, describe, expect, it } from 'vitest'; +import './query-condition-row'; +import type { FilterCondition } from './query-types'; +import type { ProtspaceData } from './types'; + +interface ConditionRowEl extends HTMLElement { + condition: FilterCondition; + annotations: string[]; + data: ProtspaceData; + updateComplete: Promise; + _selectAnnotation(annotation: string): void; +} + +const data: ProtspaceData = { + protein_ids: ['P1', 'P2'], + annotations: { + organism: { kind: 'categorical', values: ['Human', 'Mouse'] }, + // A genuine, unmaterialized numeric annotation. + length: { kind: 'numeric', values: [] }, + // A numeric annotation after legend materialization: binned to categorical, + // but still numeric at heart (sourceKind). + mass: { kind: 'categorical', sourceKind: 'numeric', values: ['0–10', '10–20'] }, + }, + annotation_data: { organism: [[0], [1]] }, + numeric_annotation_data: { length: [100, 200], mass: [5, 15] }, +}; + +async function mount(): Promise { + document.body.innerHTML = ''; + const el = document.createElement('protspace-query-condition-row') as ConditionRowEl; + el.condition = { id: 'c1', kind: 'categorical', annotation: '', values: [] }; + el.annotations = ['organism', 'length', 'mass']; + el.data = data; + document.body.appendChild(el); + await el.updateComplete; + return el; +} + +async function kindAfterSelecting(el: ConditionRowEl, annotation: string): Promise { + let kind = ''; + el.addEventListener( + 'condition-changed', + (e) => { + kind = (e as CustomEvent<{ condition: FilterCondition }>).detail.condition.kind; + }, + { once: true }, + ); + el._selectAnnotation(annotation); + await el.updateComplete; + return kind; +} + +describe('query-condition-row annotation kind detection', () => { + beforeEach(() => { + document.body.innerHTML = ''; + }); + + it('builds a categorical condition for a categorical annotation', async () => { + const el = await mount(); + expect(await kindAfterSelecting(el, 'organism')).toBe('categorical'); + }); + + it('builds a numeric condition for a numeric annotation', async () => { + const el = await mount(); + expect(await kindAfterSelecting(el, 'length')).toBe('numeric'); + }); + + it('builds a numeric condition for a materialized (sourceKind:numeric) annotation', async () => { + const el = await mount(); + expect(await kindAfterSelecting(el, 'mass')).toBe('numeric'); + }); +}); diff --git a/packages/core/src/components/control-bar/query-condition-row.ts b/packages/core/src/components/control-bar/query-condition-row.ts index 62ccd5b5..f9cc0ef2 100644 --- a/packages/core/src/components/control-bar/query-condition-row.ts +++ b/packages/core/src/components/control-bar/query-condition-row.ts @@ -5,7 +5,7 @@ import type { FilterCondition, LogicalOp, NumericCondition } from './query-types import { createCondition, createNumericCondition } from './query-types'; import type { ProtspaceData } from './types'; import { groupAnnotations } from './annotation-categories'; -import { NA_VALUE, NA_DISPLAY } from '@protspace/utils'; +import { NA_VALUE, NA_DISPLAY, isNumericAnnotation } from '@protspace/utils'; import { queryBuilderStyles } from './query-builder.styles'; import './query-value-picker'; import './query-numeric-input'; @@ -120,7 +120,11 @@ class ProtspaceQueryConditionRow extends LitElement { logicalOp: this.condition.logicalOp, annotation, }; - const isNumeric = this.data?.annotations?.[annotation]?.kind === 'numeric'; + // Use the shared numeric check so that a numeric annotation is still treated as + // numeric after materialization bins it to kind:'categorical' (it keeps + // sourceKind:'numeric'); otherwise the currently-coloured numeric annotation + // would offer a categorical bin picker instead of the numeric range input. + const isNumeric = isNumericAnnotation(this.data?.annotations?.[annotation]); this._dispatchChanged(isNumeric ? createNumericCondition(base) : createCondition(base)); } diff --git a/packages/core/src/components/control-bar/query-evaluate.test.ts b/packages/core/src/components/control-bar/query-evaluate.test.ts index ead00264..927588bf 100644 --- a/packages/core/src/components/control-bar/query-evaluate.test.ts +++ b/packages/core/src/components/control-bar/query-evaluate.test.ts @@ -271,11 +271,27 @@ describe('evaluateQuery', () => { expect(result).toEqual(new Set([0, 1, 2, 3])); }); - it('matches nothing when a required bound is missing', () => { + it('is a no-op (matches all) when a required bound is missing, like an empty categorical condition', () => { const query: FilterQuery = [ { id: '1', kind: 'numeric', annotation: 'length', operator: 'gt', min: null, max: null }, ]; const result = evaluateQuery(query, createTestData()); + expect(result).toEqual(new Set([0, 1, 2, 3, 4])); + }); + + it('matches nothing when an unconfigured numeric condition is negated (symmetric with empty categorical)', () => { + const query: FilterQuery = [ + { + id: '1', + logicalOp: 'NOT', + kind: 'numeric', + annotation: 'length', + operator: 'gt', + min: null, + max: null, + }, + ]; + const result = evaluateQuery(query, createTestData()); expect(result).toEqual(new Set()); }); diff --git a/packages/core/src/components/control-bar/query-evaluate.ts b/packages/core/src/components/control-bar/query-evaluate.ts index 6932a9d5..954e415c 100644 --- a/packages/core/src/components/control-bar/query-evaluate.ts +++ b/packages/core/src/components/control-bar/query-evaluate.ts @@ -79,15 +79,17 @@ function evaluateCondition( /** * Evaluate a numeric condition: a protein matches when its raw numeric value - * satisfies the operator. An unconfigured condition (missing a required bound) - * matches nothing, so an in-progress filter never produces misleading results. + * satisfies the operator. An unconfigured condition (missing a required bound) is + * a no-op and matches everything — mirroring an empty categorical condition. This + * keeps the two kinds symmetric under NOT: `NOT (unconfigured)` then matches + * nothing (and leaves Apply disabled) instead of isolating every protein. */ function evaluateNumericCondition( condition: NumericCondition, data: ProtspaceData, numProteins: number, ): Set { - if (!isNumericConditionReady(condition)) return new Set(); + if (!isNumericConditionReady(condition)) return allIndices(numProteins); const values = data.numeric_annotation_data?.[condition.annotation]; if (!values) return new Set(); diff --git a/packages/core/src/components/control-bar/query-numeric-input.test.ts b/packages/core/src/components/control-bar/query-numeric-input.test.ts new file mode 100644 index 00000000..0c89efc2 --- /dev/null +++ b/packages/core/src/components/control-bar/query-numeric-input.test.ts @@ -0,0 +1,134 @@ +/** + * @vitest-environment jsdom + * + * Behavioural contract for the numeric range input used by a query condition. + * The condition object is owned by the control bar; this component is controlled, + * so each test feeds an emitted condition back in as the prop (as the real parent + * does) before asserting the next render. + */ +import { beforeEach, describe, expect, it } from 'vitest'; +import './query-numeric-input'; +import type { NumericCondition, NumericOperator } from './query-types'; + +interface NumericInputEl extends HTMLElement { + condition: NumericCondition; + data: unknown; + updateComplete: Promise; +} + +function makeCondition(overrides: Partial = {}): NumericCondition { + return { + id: 'n1', + kind: 'numeric', + annotation: 'length', + operator: 'gt', + min: null, + max: null, + ...overrides, + }; +} + +async function mount(condition: NumericCondition): Promise { + document.body.innerHTML = ''; + const el = document.createElement('protspace-query-numeric-input') as NumericInputEl; + el.condition = condition; + document.body.appendChild(el); + await el.updateComplete; + return el; +} + +function fields(el: NumericInputEl) { + const root = el.shadowRoot!; + return { + select: root.querySelector('.numeric-operator-select') as HTMLSelectElement | null, + inputs: Array.from(root.querySelectorAll('input.numeric-field')) as HTMLInputElement[], + }; +} + +/** Drive the operator @@ -133,6 +142,7 @@ class ProtspaceQueryNumericInput extends LitElement { ? html` Date: Sat, 30 May 2026 00:18:53 +0200 Subject: [PATCH 18/19] =?UTF-8?q?chore(control-bar):=20polish=20=E2=80=94?= =?UTF-8?q?=20docs,=20dead=20code,=20and=20intent-pinning=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - docs(explore): replace the removed XY/XZ/YZ plane-selector tip with a note that 3D projections render as their X/Y view (completes the #196 removal). - test(scatter-plot): drop z:0 from style-getters mock points; PlotDataPoint.z was removed in this PR. - refactor(query-types): stop seeding createGroup with a leading logicalOp:'AND' so a first-position group can't carry a spurious operator (mirrors createCondition; the builder sets the op for non-first groups). - test(control-bar): pin the flat left-to-right operator precedence ((A OR B) AND C) so the known limitation stays an intentional choice. --- docs/explore/control-bar.md | 2 +- .../control-bar/query-evaluate.test.ts | 30 +++++++++++++++++++ .../src/components/control-bar/query-types.ts | 3 +- .../scatter-plot/style-getters.test.ts | 3 -- 4 files changed, 33 insertions(+), 5 deletions(-) diff --git a/docs/explore/control-bar.md b/docs/explore/control-bar.md index 2e5f7bde..a3032904 100644 --- a/docs/explore/control-bar.md +++ b/docs/explore/control-bar.md @@ -25,7 +25,7 @@ Your current projection is reflected in the page URL, so refresh, browser back/f ::: ::: tip 3D Projections -When a 3D projection is available, a **plane selector** (XY / XZ / YZ) appears, letting you view different 2D slices of the 3D space. +3D projections load normally and are shown as their X/Y view; the third (Z) dimension is not rendered in the web viewer. ::: ## 2. Annotation Selector diff --git a/packages/core/src/components/control-bar/query-evaluate.test.ts b/packages/core/src/components/control-bar/query-evaluate.test.ts index 927588bf..83622a10 100644 --- a/packages/core/src/components/control-bar/query-evaluate.test.ts +++ b/packages/core/src/components/control-bar/query-evaluate.test.ts @@ -161,6 +161,36 @@ describe('evaluateQuery', () => { }); }); + describe('operator precedence (flat, left-to-right)', () => { + // Conditions are folded strictly left-to-right with no AND-over-OR precedence: + // `A OR B AND C` evaluates as `(A OR B) AND C`, not the SQL-conventional + // `A OR (B AND C)`. Use groups for the latter. This test pins that behaviour so + // it stays an intentional choice rather than an accidental regression. + it('folds A OR B AND C as (A OR B) AND C', () => { + const query: FilterQuery = [ + { id: '1', kind: 'categorical', annotation: 'organism', values: ['Zebrafish'] }, + { + id: '2', + logicalOp: 'OR', + kind: 'categorical', + annotation: 'organism', + values: ['Mouse'], + }, + { + id: '3', + logicalOp: 'AND', + kind: 'categorical', + annotation: 'reviewed', + values: ['false'], + }, + ]; + const result = evaluateQuery(query, createTestData()); + // (Zebrafish{3} OR Mouse{1,4}) AND reviewed=false{1,4} -> {1,4} + // NOT Zebrafish{3} OR (Mouse{1,4} AND reviewed=false{1,4}) -> {1,3,4} + expect(result).toEqual(new Set([1, 4])); + }); + }); + describe('groups', () => { it('evaluates a group as a unit', () => { const query: FilterQuery = [ diff --git a/packages/core/src/components/control-bar/query-types.ts b/packages/core/src/components/control-bar/query-types.ts index 4d424aec..47e9a290 100644 --- a/packages/core/src/components/control-bar/query-types.ts +++ b/packages/core/src/components/control-bar/query-types.ts @@ -61,7 +61,8 @@ export function createNumericCondition(overrides?: Partial): N export function createGroup(overrides?: Partial): FilterGroup { return { id: generateId(), - logicalOp: 'AND', + // No leading logicalOp by default, mirroring createCondition — the builder + // sets one only for a group that is not first (query-builder._addGroup). conditions: [createCondition()], ...overrides, }; diff --git a/packages/core/src/components/scatter-plot/style-getters.test.ts b/packages/core/src/components/scatter-plot/style-getters.test.ts index e9164377..7ff5cbf9 100644 --- a/packages/core/src/components/scatter-plot/style-getters.test.ts +++ b/packages/core/src/components/scatter-plot/style-getters.test.ts @@ -32,7 +32,6 @@ describe('style-getters', () => { id, x: 0, y: 0, - z: 0, originalIndex, }); @@ -183,7 +182,6 @@ describe('style-getters', () => { id, x: 0, y: 0, - z: 0, originalIndex, }); @@ -323,7 +321,6 @@ describe('style-getters', () => { id: 'test_protein', x: 0, y: 0, - z: 0, originalIndex, }); From 3d55218956144aae3688efe1d4f596ee36c7139a Mon Sep 17 00:00:00 2001 From: peymanvahidi Date: Sat, 30 May 2026 16:03:47 +0200 Subject: [PATCH 19/19] fix(scatter-plot): keep global originalIndex under a query filter (#257 follow-up) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Routing query filters through the filteredProteinIds channel (df07c96) exposed a latent rendering bug. _processData built _plotData from _getCurrentDisplayData(), which COMPACTS the matched subset, so each PlotDataPoint.originalIndex became a slice-local 0..N index. But the style getters (_buildStyleGetters uses the full materialized data) and the tooltip path (this.data) resolve annotation values by originalIndex against the FULL dataset. For any non-prefix filter (e.g. protein_family=X, which keeps a scattered subset) every kept point was painted with the WRONG protein's colour/shape/opacity and showed the wrong tooltip — and points whose mis-resolved value was legend-hidden vanished. The channel was dormant before df07c96, so query-apply is what first made this reachable. Fix: build _plotData from the full materialized data and apply the matched-id set as a membership filter AFTER the map (a new optional `visibleProteinIds` arg on DataProcessor.processVisualizationData), exactly as isolation already does, so originalIndex stays a GLOBAL index. The style getters and tooltips — both written for global indices — now resolve correctly with no change. The fast path is gated off whenever a filter is active (it already rebuilt every time under the old slice-by-reference approach, so no perf change). getCurrentData()/data-change keep slicing for the legend and export, which want the filtered protein list. Add scatter-plot.filter-render.test.ts pinning per-point colour for a non-prefix filter, plus the prefix and unfiltered cases. --- .../scatter-plot.filter-render.test.ts | 117 ++++++++++++++++++ .../components/scatter-plot/scatter-plot.ts | 22 +++- .../utils/src/visualization/data-processor.ts | 19 ++- 3 files changed, 149 insertions(+), 9 deletions(-) create mode 100644 packages/core/src/components/scatter-plot/scatter-plot.filter-render.test.ts diff --git a/packages/core/src/components/scatter-plot/scatter-plot.filter-render.test.ts b/packages/core/src/components/scatter-plot/scatter-plot.filter-render.test.ts new file mode 100644 index 00000000..d36feca5 --- /dev/null +++ b/packages/core/src/components/scatter-plot/scatter-plot.filter-render.test.ts @@ -0,0 +1,117 @@ +/** + * @vitest-environment jsdom + * + * Rendering-integrity regression for the query-filter channel (#257 follow-up). + * + * When a query filter is active, the scatter plot builds `_plotData` from the + * matched subset. Each PlotDataPoint.originalIndex MUST still address the full + * dataset, because the style getters (colors/shape/opacity) and the tooltip + * path resolve annotation values against the full data by that index — exactly + * as the isolation path already does. If a filter renumbers originalIndex to a + * slice-local 0..N-1, then a non-prefix filter (one that drops earlier proteins) + * paints kept points with the WRONG protein's colour and can even hide them. + * + * Construct the element via createElement without appending it (so Lit's + * connectedCallback / WebGL init never runs — same approach as + * scatter-plot.isolation.test.ts) and drive _processData directly. + */ +import { vi, describe, it, expect } from 'vitest'; +import type { PlotDataPoint, VisualizationData } from '@protspace/utils'; + +vi.hoisted(() => { + if (!('ResizeObserver' in globalThis)) { + (globalThis as unknown as { ResizeObserver: unknown }).ResizeObserver = class { + observe() {} + unobserve() {} + disconnect() {} + }; + } +}); + +import './scatter-plot'; + +const RED = '#ff0000'; +const GREEN = '#00ff00'; + +type ScatterplotInternals = HTMLElement & { + data: VisualizationData; + selectedAnnotation: string; + filteredProteinIds: string[]; + filtersActive: boolean; + _plotData: PlotDataPoint[]; + _processData(): void; + _buildStyleGetters(): { getColors(point: PlotDataPoint): string[] }; +}; + +/** + * Six proteins. p0–p2 are family "A" (red), p3–p5 are family "B" (green). + * annotation_data rows hold an index into `annotations.fam.values`, and + * valueToColor is derived from values↔colors positionally, so A→red, B→green. + */ +function makeFamilyData(): VisualizationData { + const families = ['A', 'A', 'A', 'B', 'B', 'B']; + const colorFor = (v: string) => (v === 'A' ? RED : GREEN); + return { + protein_ids: families.map((_, i) => `p${i}`), + projections: [{ name: 'umap', data: families.map((_, i) => [i, i]) }], + annotations: { + fam: { + values: families, + colors: families.map(colorFor), + shapes: families.map(() => 'circle'), + }, + }, + annotation_data: { + // each row points at the first index of its family value in `values` + fam: families.map((v) => [families.indexOf(v)]), + }, + } as unknown as VisualizationData; +} + +function makeScatter(): ScatterplotInternals { + const sp = document.createElement('protspace-scatterplot') as ScatterplotInternals; + sp.data = makeFamilyData(); + sp.selectedAnnotation = 'fam'; + return sp; +} + +describe('scatter-plot query-filter rendering integrity', () => { + it('colours a non-prefix filtered subset by each point’s OWN value', () => { + const sp = makeScatter(); + // Keep only family B — a non-prefix subset (drops p0–p2). + sp.filteredProteinIds = ['p3', 'p4', 'p5']; + sp.filtersActive = true; + + sp._processData(); + const getters = sp._buildStyleGetters(); + + expect(sp._plotData.map((p) => p.id).sort()).toEqual(['p3', 'p4', 'p5']); + + // Every kept point is family B → must be green, not the colour of the + // protein sitting at the same slice-local position in the full dataset. + for (const point of sp._plotData) { + expect(getters.getColors(point)).toEqual([GREEN]); + } + }); + + it('still colours a prefix filter and the unfiltered plot correctly', () => { + // Prefix subset (p0–p2, family A) — happens to work even with the bug, so + // this guards against an over-correction that breaks the easy case. + const prefix = makeScatter(); + prefix.filteredProteinIds = ['p0', 'p1', 'p2']; + prefix.filtersActive = true; + prefix._processData(); + const prefixGetters = prefix._buildStyleGetters(); + for (const point of prefix._plotData) { + expect(prefixGetters.getColors(point)).toEqual([RED]); + } + + // No filter at all — full plot, both families correct. + const full = makeScatter(); + full._processData(); + const fullGetters = full._buildStyleGetters(); + const colorById = new Map(full._plotData.map((p) => [p.id, fullGetters.getColors(p)])); + expect(colorById.get('p0')).toEqual([RED]); + expect(colorById.get('p5')).toEqual([GREEN]); + }); +}); diff --git a/packages/core/src/components/scatter-plot/scatter-plot.ts b/packages/core/src/components/scatter-plot/scatter-plot.ts index 1a4a6526..9124ecac 100644 --- a/packages/core/src/components/scatter-plot/scatter-plot.ts +++ b/packages/core/src/components/scatter-plot/scatter-plot.ts @@ -583,14 +583,25 @@ export class ProtspaceScatterplot extends LitElement { } private _processData() { - const dataToUse = this._getCurrentDisplayData(); + // Build _plotData from the FULL materialized data and apply the query filter + // as an id-membership filter (see processVisualizationData) rather than from a + // pre-sliced display array. This keeps each point's originalIndex a GLOBAL + // index into the full dataset, which the style getters and tooltip path + // require — a slice-local index would mis-resolve colours/values under any + // non-prefix filter. Isolation already worked this way; filtering now matches. + const dataToUse = this._getMaterializedData(); if (!dataToUse) return; - // Detect whether only the projection changed (same data reference, not in isolation mode). - // In this case we take the fast path: update coordinates in-place - // instead of rebuilding all PlotDataPoint objects with annotation data (~700MB for 500k proteins). + const visibleProteinIds = this._getVisibleProteinIdsSet(); + + // Fast path applies only to a projection change on the plain, unfiltered, + // non-isolated plot. Whenever a filter or isolation is active we rebuild so + // the kept set is recomputed (and originalIndex stays global). const onlyProjectionChanged = - this._plotData.length > 0 && this._lastDataRef === dataToUse && !this._isolationMode; + this._plotData.length > 0 && + this._lastDataRef === dataToUse && + !this._isolationMode && + !this.filtersActive; if (onlyProjectionChanged) { // Fast path: update coordinates in-place from the new projection data. @@ -610,6 +621,7 @@ export class ProtspaceScatterplot extends LitElement { this.selectedProjectionIndex, this._isolationMode, this._isolationHistory, + visibleProteinIds, ); } diff --git a/packages/utils/src/visualization/data-processor.ts b/packages/utils/src/visualization/data-processor.ts index 47cab637..94cd290e 100644 --- a/packages/utils/src/visualization/data-processor.ts +++ b/packages/utils/src/visualization/data-processor.ts @@ -7,6 +7,7 @@ export class DataProcessor { projectionIndex: number, isolationMode: boolean = false, isolationHistory?: string[][], + visibleProteinIds?: Set | null, ): PlotDataPoint[] { if (!data.projections[projectionIndex]) return []; @@ -18,16 +19,26 @@ export class DataProcessor { return { id, x: coordinates[0], y: coordinates[1], originalIndex: index }; }); + // Apply the query filter (and then isolation) as id-membership filters AFTER + // the map, so every kept point keeps its GLOBAL `originalIndex`. Style getters + // and tooltips resolve annotation values against the full dataset by that + // index, so a slice-local index would mis-resolve points under a non-prefix + // filter. This mirrors how isolation has always preserved the global index. + let result = processedData; + + if (visibleProteinIds) { + result = result.filter((p) => visibleProteinIds.has(p.id)); + } + if (isolationMode && isolationHistory && isolationHistory.length > 0) { - let filteredData = processedData.filter((p) => isolationHistory[0].includes(p.id)); + result = result.filter((p) => isolationHistory[0].includes(p.id)); for (let i = 1; i < isolationHistory.length; i++) { const splitIds = isolationHistory[i]; - filteredData = filteredData.filter((p) => splitIds.includes(p.id)); + result = result.filter((p) => splitIds.includes(p.id)); } - return filteredData; } - return processedData; + return result; } static createScales(