fix(plugin-charts,core): conjoin the chart drill filter instead of spreading it

.changeset/8944-chart-drill-filter-composition.md

@@ -0,0 +1,44 @@
+---
+'@object-ui/core': minor
+'@object-ui/plugin-charts': patch
+'@object-ui/app-shell': patch
+'@object-ui/types': patch
+---
+
+Compose an `ObjectChart` drill-down filter instead of spreading it, so the widget's own
+filter survives into the drilled query for BOTH arms of `ObjectChartSchema.filter`
+(objectui#8944).
+
+**The defect.** `ObjectChartSchema.filter` admits a spec `FilterArray`
+(`[['region','=','emea']]`) and the ObjectQL `$filter` object (`{ region: 'emea' }`),
+and both are read — both travel verbatim to `ds.aggregate` / `ds.find`. The drill seam
+composed them by spreading the widget's filter into an object literal, which is correct
+for the object arm and silent nonsense for the array arm: spreading an array yields
+index keys, so an authored `FilterArray` drilled as
+`{ '0': ['region','=','emea'], stage: 'won' }` — the widget's conditions replaced by a
+key the query layer ignores. Nothing errored; the drawer opened and looked right.
+
+**Direction of the failure.** The widget's filter is what narrows. Dropping it made the
+drilled list a **superset** — it showed records the chart itself was scoped to exclude.
+Not a security boundary, but the worse direction for a silent bug.
+
+**The composition rule, named rather than picked.** `widget.filter ∧ drill.filter`. The
+two are independent filter sources and a drill must satisfy both: the click context only
+says which bucket of the widget's scope was asked for, so it may narrow that scope and
+never widen it. This is not a new rule — it is the contract `mergeFilterNodes` already
+states ("combine filter sources under a single `and`, each as its OWN child"), the sink
+every other multi-source filter in this repo goes through. A new
+`composeDrillFilter` helper in `@object-ui/core` applies it at the drill seam and
+documents it, then lowers the result back to the `FilterCondition` object dialect with
+`parseFilterAST` — the spec's single lowering sink — because that is the dialect both
+drill sinks take.
+
+**Compatibility.** A lone surviving source lowers back to exactly the flat object the
+spread produced, so a chart with no filter of its own drills byte-identically to before.
+Only a genuinely composed pair gains the `$and`.
+
+`serializeDrillFilterParams` (the drill "Open in list" / `target: 'navigate'` URL writer)
+learns to flatten that `$and` into the flat `filter[...]` params its own read side already
+ANDs back together. Without that it took the `String(value)` path — `$and` holds an array —
+and emitted `filter[$and]=[object Object],[object Object]` while both real conditions
+vanished, which is the outcome that function's contract says it never produces.

packages/app-shell/src/views/drillUrlFilters.test.ts

@@ -59,6 +59,57 @@ describe('serializeDrillFilterParams', () => {
   });
 });
 
+describe('serializeDrillFilterParams — a COMPOSED drill filter (objectui#8944)', () => {
+  /**
+   * `composeDrillFilter` lowers `widget.filter ∧ click context` to
+   * `{ $and: […] }` whenever both sources survive. Before this branch existed
+   * that value fell to the `String(value)` path — `$and` holds an ARRAY, so it
+   * was neither null nor a non-array object — and produced a bogus
+   * `filter[$and]=[object Object],[object Object]` while BOTH real conditions
+   * vanished, i.e. the list landed scoped by nothing the user clicked.
+   */
+  it('flattens a top-level $and into the params of each child', () => {
+    const qs = serializeDrillFilterParams({ $and: [{ region: 'emea' }, { stage: 'won' }] });
+    expect(qs.get('filter[region]')).toBe('emea');
+    expect(qs.get('filter[stage]')).toBe('won');
+    // The hazard, named: no key spells the combinator, and nothing stringified.
+    expect(qs.get('filter[$and]')).toBeNull();
+    expect(qs.toString()).not.toContain('object%20Object');
+  });
+
+  it('walks a NESTED $and, which is what composing an array arm produces', () => {
+    // `[['stage','=','won'],['amount','>',100]]` conjoined with a click context
+    // lowers to an $and whose first child is itself an $and.
+    const qs = serializeDrillFilterParams({
+      $and: [{ $and: [{ stage: 'won' }, { amount: { $gt: 100 } }] }, { region: 'emea' }],
+    });
+    expect(qs.get('filter[stage]')).toBe('won');
+    expect(qs.get('filter[amount][gt]')).toBe('100');
+    expect(qs.get('filter[region]')).toBe('emea');
+  });
+
+  it('a composed filter survives the URL round-trip as a conjunction of triples', () => {
+    // The read side ANDs its triples, so the conjunction is preserved in
+    // meaning, not just in bytes.
+    const triples = parseUrlFilterTriples(
+      serializeDrillFilterParams({
+        $and: [{ region: 'emea' }, { close_date: { $gte: '2026-06-01', $lt: '2026-07-01' } }],
+      }),
+    );
+    expect(triples).toEqual<FilterTriple[]>([
+      ['region', '=', 'emea'],
+      ['close_date', '>=', '2026-06-01'],
+      ['close_date', '<', '2026-07-01'],
+    ]);
+  });
+
+  it('skips a bare ARRAY comparand rather than stringifying it', () => {
+    // The same promise the unknown-object case makes, for the shape that used
+    // to escape it.
+    expect(serializeDrillFilterParams({ tags: ['a', 'b'] }).toString()).toBe('');
+  });
+});
+
 describe('round-trip: serialize → parse (write and read sides agree)', () => {
   it('a mixed equality + date-range drill filter survives the URL round-trip', () => {
     const filter = { stage: 'qualification', close_date: { $gte: '2026-06-01', $lt: '2026-07-01' } };

packages/app-shell/src/views/drillUrlFilters.ts

@@ -57,24 +57,65 @@ export function parseUrlFilterTriples(searchParams: URLSearchParams): FilterTrip
  * plain value becomes `filter[field]`. `null`/`undefined` values and objects
  * with no recognized operator are skipped (drill degrades to a superset) rather
  * than stringified to `"[object Object]"`.
+ *
+ * ## `$and` is flattened, because this dialect's conjunction is implicit
+ *
+ * A drill filter composed from more than one source arrives as
+ * `{ $and: [<widget filter>, <click context>] }` — what `composeDrillFilter`
+ * (`@object-ui/core`) lowers `widget.filter ∧ drill.filter` to (objectui#8944).
+ * The READ side already returns a FLAT list of triples that the query layer ANDs
+ * together, so a top-level `$and` is expressible here: emit each child's params
+ * into the same set and `parseUrlFilterTriples` reads the conjunction straight
+ * back. Nesting is walked too, since composing three sources nests.
+ *
+ * ⚠️ Without this branch a composed filter took the `String(value)` path below —
+ * `$and` holds an ARRAY, so it was neither `null` nor a non-array object — and
+ * the URL grew a bogus `filter[$and]=[object Object],[object Object]` while BOTH
+ * real conditions vanished. That is the very outcome this function's contract
+ * says it never produces, and it is wrong in the widening direction: the list
+ * lands unscoped by anything the user actually clicked.
+ *
+ * ⚠️ Two conditions on the SAME field and operator are NOT expressible here (one
+ * param key, one value). The later source wins, so a click context still
+ * overrides the widget's condition on that field exactly as it did when this
+ * value was built by spreading — the drill degrades to a superset there, the
+ * same posture this function already takes for operators it cannot spell.
  */
 export function serializeDrillFilterParams(
   filter: Record<string, unknown> | undefined,
 ): URLSearchParams {
   const params = new URLSearchParams();
   if (!filter) return params;
+  collectFilterParams(filter, params);
+  return params;
+}
+
+/** One source's conditions, written into the shared param set. Recurses on `$and`. */
+function collectFilterParams(filter: Record<string, unknown>, params: URLSearchParams): void {
   for (const [field, value] of Object.entries(filter)) {
     if (value == null) continue;
+    if (field === '$and' && Array.isArray(value)) {
+      // Implicit-AND dialect: each child contributes its own params, in order,
+      // so a later source overrides an earlier one on a field they share.
+      for (const child of value) {
+        if (child && typeof child === 'object' && !Array.isArray(child)) {
+          collectFilterParams(child as Record<string, unknown>, params);
+        }
+      }
+      continue;
+    }
     if (typeof value === 'object' && !Array.isArray(value)) {
       for (const [op, suffix] of Object.entries(RANGE_OP_PARAM)) {
         const bound = (value as Record<string, unknown>)[op];
         if (bound != null) params.set(`filter[${field}][${suffix}]`, String(bound));
       }
       continue; // handled (range ops) or skipped — never String(object)
     }
+    // Arrays reach here as `$in`-style comparands this dialect cannot spell;
+    // skipping keeps the promise above (never `String(array)`).
+    if (Array.isArray(value)) continue;
     params.set(`filter[${field}]`, String(value));
   }
-  return params;
 }
 
 /**

packages/core/src/utils/drill-down.ts

@@ -22,8 +22,11 @@
  * synthesize an `event` object and rely on the same defaults / templating.
  */
 
+import { parseFilterAST } from '@objectstack/spec/data';
 import type { DrillDownConfig } from '@object-ui/types';
 
+import { mergeFilterNodes } from './filter-converter.js';
+
 /**
  * Generic click payload. Pivots provide row/col, charts provide
  * category/series. Extra fields are passed through verbatim so callers
@@ -183,3 +186,72 @@ export function isDrillEnabled(config: DrillDownConfig | undefined): boolean {
   if (!config) return false;
   return config.enabled !== false;
 }
+
+/**
+ * Compose a widget's OWN filter with the filter a drill click derived, into the
+ * one filter the drilled list is scoped by.
+ *
+ * ## The rule: `widget.filter ∧ drill.filter`
+ *
+ * The two are independent filter SOURCES and a drill must satisfy BOTH. The
+ * widget's filter is what scopes the chart; the click context only says WHICH
+ * bucket of that scope the user asked to see. So a drill may narrow the widget's
+ * scope and may never widen it — which makes the composition a conjunction, not
+ * a merge and emphatically not a spread.
+ *
+ * ⛔ The rule is NOT invented here. It is the contract {@link mergeFilterNodes}
+ * already states — "combine filter sources under a single `and`, each as its OWN
+ * child" — the sink every other multi-source filter in this repo goes through
+ * (`ObjectView`, `RelatedList`, `LineItemsPanel`, `RecordPickerDialog`,
+ * `ElementDataSourceGate`, `buildEffectiveFilter`). This function only applies
+ * it at the drill seam and names it, so the answer is in one place rather than
+ * re-derived per widget.
+ *
+ * ## Why the two arms needed a sink at all (objectui#8944)
+ *
+ * `ObjectChartSchema.filter` admits BOTH a spec `FilterArray`
+ * (`[['stage','=','won']]`) and the ObjectQL `$filter` object
+ * (`{ close_date: { $gte } }`), because both are read — both are forwarded
+ * verbatim to `ds.aggregate` / `ds.find`. The drill seam used to compose them by
+ * SPREADING the widget's filter into an object literal, which is correct for the
+ * object arm and silent nonsense for the array arm: spreading `[['stage','=',
+ * 'won']]` yields the index key `{ '0': ['stage','=','won'] }`, so the widget's
+ * own conditions were replaced by a key the query layer ignores and the drilled
+ * list showed rows the chart itself was scoped to exclude. `toFilterNode` (via
+ * `mergeFilterNodes`) already lowers all three shapes in circulation, so routing
+ * the pair through it is what makes the array arm survive.
+ *
+ * ## Why the result is lowered back to the object dialect
+ *
+ * {@link mergeFilterNodes} answers in the ObjectQL AST
+ * (`['and', <widget>, <drill>]`). Both drill sinks take the `FilterCondition`
+ * OBJECT dialect instead — the drawer hands the value to `object-data-table`'s
+ * `filter` (which becomes `$filter`), and `DrillNavigationContext.openRecordList`
+ * declares `Record<string, unknown>` and serializes it to `filter[...]` URL
+ * params. `parseFilterAST` is the spec's single lowering sink between the two
+ * dialects, so it is what converts, rather than a second local translation.
+ *
+ * ⭐ A lone surviving source lowers back to exactly the flat object the spread
+ * produced (`{ stage: 'won' }`), so a chart with no filter of its own drills
+ * identically to before; only a genuinely composed pair gains the `$and`.
+ *
+ * Returns `undefined` when neither source carries anything, so callers can omit
+ * the key rather than send an empty filter.
+ *
+ * ⚠️ Refusals are the sink's, not this seam's: `mergeFilterNodes` rejects a
+ * comparand the wire would also reject (a bare array on `=`, a `RegExp`) with
+ * the `INVALID_FILTER` / 400 envelope. Such a filter already fails the widget's
+ * OWN query for the same reason, so the drill and the chart now agree instead of
+ * the drill quietly sending something the chart could not.
+ */
+export function composeDrillFilter(
+  widgetFilter: unknown,
+  drillFilter: Record<string, unknown> | undefined,
+): Record<string, unknown> | undefined {
+  // `FilterCondition` is the spec's object-dialect filter; the drill sinks type
+  // the same value as `Record<string, unknown>`, and this is the one seam where
+  // the two names meet.
+  return parseFilterAST(mergeFilterNodes(widgetFilter, drillFilter)) as
+    | Record<string, unknown>
+    | undefined;
+}

packages/plugin-charts/src/ObjectChart.drillNavigate.test.tsx

@@ -133,9 +133,15 @@ describe("ObjectChart — DrillDownConfig.target: 'navigate' (objectui#3354)", (
 
     fireEvent.click(screen.getByTestId('fake-segment'));
 
-    // Widget filter ∧ click context — the same merge the drawer would have used.
+    // Widget filter ∧ click context — the same composition the drawer would have
+    // used. The conjunction was always the intent this comment stated; since
+    // objectui#8944 it is spelled by the repo's single filter sink
+    // (`composeDrillFilter`) instead of by spreading the widget's filter into an
+    // object literal, which only worked when that filter was the object arm.
     await waitFor(() =>
-      expect(openRecordList).toHaveBeenCalledWith('opportunity', { owner: 'me', stage: 'won' }),
+      expect(openRecordList).toHaveBeenCalledWith('opportunity', {
+        $and: [{ owner: 'me' }, { stage: 'won' }],
+      }),
     );
     expect(screen.queryByTestId('chart-drill-body')).toBeNull();
     // …and it does not fire again on subsequent renders.

packages/plugin-charts/src/ObjectChart.tsx

@@ -3,7 +3,7 @@ import React, { useState, useEffect, useContext, useCallback, useMemo } from 're
 import { useDataScope, SchemaRendererContext, SchemaRenderer, useDrillNavigation, useFilterScope, ElementDataSourceGate, type ElementDataSourceMapping } from '@object-ui/react';
 import { ChartRenderer } from './ChartRenderer';
 import { normalizeChartSchema } from './normalizeChartSchema';
-import { ComponentRegistry, chartMeasureKey, humanizeLabel, extractRecords, computeDrillFilter, isDrillEnabled, resolveDrillTitle, resolveFilterPlaceholders, resolveContextTokens, shiftFilterByCompareTo, compareToTrendLabelKey, buildChartSeries, buildOptionColorMap, deriveDimensionLabelMaps, dimensionOptionTranslator, loadDimensionFieldMeta, relabelDimensions, localizeFieldOptions, elementDataSourceBlock, type DimensionFieldMeta, type CompareToConfig, type DrillEvent, type ChartResultField, type ChartSegmentClickEvent } from '@object-ui/core';
+import { ComponentRegistry, chartMeasureKey, humanizeLabel, extractRecords, computeDrillFilter, composeDrillFilter, isDrillEnabled, resolveDrillTitle, resolveFilterPlaceholders, resolveContextTokens, shiftFilterByCompareTo, compareToTrendLabelKey, buildChartSeries, buildOptionColorMap, deriveDimensionLabelMaps, dimensionOptionTranslator, loadDimensionFieldMeta, relabelDimensions, localizeFieldOptions, elementDataSourceBlock, type DimensionFieldMeta, type CompareToConfig, type DrillEvent, type ChartResultField, type ChartSegmentClickEvent } from '@object-ui/core';
 import { Sheet, SheetContent, SheetHeader, SheetTitle, Dialog, DialogContent, DialogHeader, DialogTitle, RefreshIndicator, Button, ChartSkeleton, DataEmptyState } from '@object-ui/components';
 import { AlertCircle, ArrowUpRight, Inbox } from 'lucide-react';
 import { builtinAggregateLabels, useSafeFieldLabel, useSafeTranslate, useObjectTranslation, pickLocalized } from '@object-ui/i18n';
@@ -1007,12 +1007,23 @@ export const ObjectChart = (props: ObjectChartProps) => {
   // target needs it from an effect, and effects may not live after an early
   // return. The drawer reads the same value, so both targets drill by exactly
   // one filter.
+  //
+  // ⛔ Composed through `composeDrillFilter`, NOT by spreading the widget's
+  // filter into an object literal. `schema.filter` admits two arms — a spec
+  // `FilterArray` and the ObjectQL `$filter` object, both of them read (both go
+  // to `ds.aggregate` / `ds.find` verbatim above) — and a spread is only correct
+  // for the second. Spreading the ARRAY arm produced index keys
+  // (`{ '0': ['stage','=','won'] }`), so the widget's own conditions were
+  // dropped for a key the query layer ignores and the drilled list showed rows
+  // this chart is scoped to exclude (objectui#8944). The seam's docblock names
+  // the composition rule (`widget.filter ∧ drill.filter`, via the repo's single
+  // filter sink `mergeFilterNodes`); it is not decided here.
   const drillFilter = useMemo(() => {
     if (!drillEvent) return undefined;
-    return {
-      ...(schema.filter || {}),
-      ...computeDrillFilter(drillDown, drillEvent, { groupByField }),
-    };
+    return composeDrillFilter(
+      schema.filter,
+      computeDrillFilter(drillDown, drillEvent, { groupByField }),
+    );
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [drillEvent, drillDown, groupByField, filterKey]);