Fix/Filter removal freeze

packages/pxweb2/src/app/components/FilterSidebar/FilterSidebar.tsx

@@ -26,6 +26,10 @@ interface FilterSidebarProps {
   onFilterChange?: () => void;
 }
 
+// Handles one subject node in the tree.
+// Checked means this node is selected as the representative filter.
+// Unchecked means this node and its descendants are removed, then the nearest
+// ancestor is restored to keep parent-level selection stable.
 const Collapsible: React.FC<CollapsibleProps> = ({
   subject,
   index,
@@ -47,11 +51,13 @@ const Collapsible: React.FC<CollapsibleProps> = ({
         value={isActive}
         subtle={!isActive && count === 0}
         onChange={(value) => {
+          // We always compute relatives from the static tree so the update logic
+          // remains deterministic regardless of current active filter state.
           const ancestors = findAncestors(subjectTree, subject.uniqueId!);
           const children = getAllDescendants(subject);
 
           if (value) {
-            // If subject has children, add the subject itself
+            // Selecting a node makes that node the explicit filter.
             dispatch({
               type: ActionType.ADD_FILTER,
               payload: [
@@ -65,34 +71,40 @@ const Collapsible: React.FC<CollapsibleProps> = ({
               ],
             });
 
-            // If the subject has children, we remove all ancestors from filter
-            for (const ancestor of ancestors) {
-              const isAncestorInFilter = state.activeFilters.some(
-                (f) => f.type === 'subject' && f.value === ancestor.id,
-              );
-              if (isAncestorInFilter) {
-                dispatch({
-                  type: ActionType.REMOVE_FILTER,
-                  payload: { value: ancestor.id, type: 'subject' },
-                });
-              }
-            }
-          } else {
-            //Remove subject and all its descendants from filter
-            const descendants = [subject, ...children];
+            // Remove any selected ancestors so we do not keep both broad and
+            // narrow subject filters active at the same time.
+            const ancestorPayload = ancestors
+              .filter((ancestor) =>
+                state.activeFilters.some(
+                  (f) => f.type === 'subject' && f.value === ancestor.id,
+                ),
+              )
+              .map((ancestor) => ({
+                value: ancestor.id,
+                type: 'subject' as const,
+              }));
 
-            for (const d of descendants) {
+            if (ancestorPayload.length > 0) {
               dispatch({
-                type: ActionType.REMOVE_FILTER,
-                payload: {
-                  value: d.id,
-                  type: 'subject',
-                  uniqueId: d.uniqueId,
-                },
+                type: ActionType.REMOVE_FILTERS,
+                payload: ancestorPayload,
               });
             }
+          } else {
+            // Deselecting a node clears that node and every descendant.
+            const descendants = [subject, ...children];
+
+            dispatch({
+              type: ActionType.REMOVE_FILTERS,
+              payload: descendants.map((d) => ({
+                value: d.id,
+                type: 'subject' as const,
+                uniqueId: d.uniqueId,
+              })),
+            });
 
-            // Ensure first parent is actually added as a filter, and not just ephemerally selected
+            // Restore nearest ancestor if needed so parent-level selection
+            // remains explicit in the active filter list.
             const parent: PathItem | undefined = ancestors.length
               ? ancestors[ancestors.length - 1]
               : undefined;

packages/pxweb2/src/app/context/FilterContext.spec.tsx

@@ -51,6 +51,8 @@ vi.mock('../util/startPageFilters', () => {
 });
 
 vi.mock('../util/tableHandler', () => ({
+  buildCompiledMatcher: (filters: Filter[]) => filters,
+  shouldTableBeIncludedWithMatcher: () => true,
   shouldTableBeIncluded: () => true,
 }));
 

packages/pxweb2/src/app/context/FilterContext.tsx

@@ -23,7 +23,10 @@ import {
   recomputeAvailableFilters,
   getStatus,
 } from '../util/startPageFilters';
-import { shouldTableBeIncluded } from '../util/tableHandler';
+import {
+  buildCompiledMatcher,
+  shouldTableBeIncludedWithMatcher,
+} from '../util/tableHandler';
 import { wrapWithLocalizedQuotemarks } from '../util/utils';
 import { Table } from 'packages/pxweb2-api-client/src';
 
@@ -67,6 +70,8 @@ export const FilterProvider: React.FC<FilterProviderProps> = ({
   );
 };
 
+// Returns the table set that should be used as the filter base.
+// When a query is active, all non-query filters are applied on top of that subset.
 function getAvailableTables(state: StartPageState): Table[] {
   if (state.availableTablesWhenQueryApplied?.length > 0) {
     return state.availableTablesWhenQueryApplied;
@@ -106,14 +111,19 @@ function reducer(
     case ActionType.ADD_FILTER: {
       const incoming = action.payload;
       const incomingTypes = new Set(incoming.map((f) => f.type));
+      // Year range behaves as a single-value filter, so old year range entries
+      // are replaced when a new year range is added.
       const clearedFilters = state.activeFilters.filter((f) =>
         incoming[0]?.type === 'yearRange' ? f.type !== 'yearRange' : true,
       );
       const newFilters = [...clearedFilters, ...incoming];
+      const matcher = buildCompiledMatcher(newFilters);
       const filteredTables = getAvailableTables(state).filter((table) =>
-        shouldTableBeIncluded(table, newFilters),
+        shouldTableBeIncludedWithMatcher(table, matcher),
       );
       const addType = action.payload[0]?.type as FilterType | undefined;
+      // lastUsedYearRange tracks the non-year constrained range and should only
+      // be recalculated when yearRange itself was not the newly added filter.
       const updatedLastUsedYearRange = incomingTypes.has('yearRange')
         ? state.lastUsedYearRange
         : getYearRanges(filteredTables);
@@ -175,8 +185,9 @@ function reducer(
             : [...state.activeFilters, newSearch];
       }
 
+      const matcher = buildCompiledMatcher(newFilters);
       const newTables = state.availableTables.filter((table) =>
-        shouldTableBeIncluded(table, newFilters),
+        shouldTableBeIncludedWithMatcher(table, matcher),
       );
 
       return {
@@ -227,18 +238,21 @@ function reducer(
       let queryTables: Table[] = [];
 
       if (action.payload.query == '') {
-        // No query => filter from all available tables
+        // No query: reset to full table set and apply remaining filters.
+        const matcher = buildCompiledMatcher(newFilters);
         newTables = state.availableTables.filter((table) =>
-          shouldTableBeIncluded(table, newFilters),
+          shouldTableBeIncludedWithMatcher(table, matcher),
         );
       } else {
-        // query present => filter from tables matching the query
+        // Query present: first narrow to API-returned table IDs, then apply
+        // all other active filters on that subset.
         queryTables = state.availableTables.filter((table) =>
           action.payload.tableIds.includes(table.id),
         );
 
+        const matcher = buildCompiledMatcher(newFilters);
         newTables = queryTables.filter((table) =>
-          shouldTableBeIncluded(table, newFilters),
+          shouldTableBeIncludedWithMatcher(table, matcher),
         );
       }
 
@@ -259,79 +273,13 @@ function reducer(
         },
       };
     }
-    case ActionType.REMOVE_FILTER: {
-      const removedType = action.payload.type;
-
-      const currentFilters =
-        removedType === 'subject' && action.payload.uniqueId
-          ? state.activeFilters.filter(
-              (filter) => filter.uniqueId !== action.payload.uniqueId,
-            )
-          : state.activeFilters.filter(
-              (filter) => filter.value !== action.payload.value,
-            );
-
-      if (currentFilters.length === 0) {
-        const fullRange = getYearRanges(state.availableTables);
-        return {
-          ...state,
-          activeFilters: [],
-          filteredTables: state.availableTables,
-          availableFilters: {
-            subjectTree: updateSubjectTreeCounts(
-              state.originalSubjectTree,
-              state.availableTables,
-            ),
-            timeUnits: getTimeUnits(state.availableTables),
-            yearRange: fullRange,
-            variables: getVariables(state.availableTables),
-            status: getStatus(state.availableTables),
-          },
-          lastUsedYearRange: fullRange,
-        };
-      }
-
-      let filteredTables: Table[] = [];
-      if (removedType === 'query') {
-        state.availableTablesWhenQueryApplied = [];
-        filteredTables = state.availableTables.filter((table) =>
-          shouldTableBeIncluded(table, currentFilters),
-        );
-      } else {
-        filteredTables = getAvailableTables(state).filter((table) =>
-          shouldTableBeIncluded(table, currentFilters),
-        );
-      }
-
-      const yearRangeStillActive = currentFilters.some(
-        (f) => f.type === 'yearRange',
-      );
-      const updatedLastUsedYearRange = yearRangeStillActive
-        ? state.lastUsedYearRange
-        : getYearRanges(filteredTables);
-
-      const recomputed = recomputeAvailableFilters(
-        removedType,
-        currentFilters,
-        getAvailableTables(state),
-        state.originalSubjectTree,
-      );
+    // Single and batch remove actions share one implementation to keep removal
+    // semantics consistent for UI events that clear one or many filters.
+    case ActionType.REMOVE_FILTER:
+      return applyRemoveFilters(state, [action.payload]);
 
-      return {
-        ...state,
-        activeFilters: currentFilters,
-        filteredTables,
-        availableFilters: {
-          subjectTree:
-            recomputed.subjectTree ?? state.availableFilters.subjectTree,
-          timeUnits: recomputed.timeUnits ?? state.availableFilters.timeUnits,
-          yearRange: recomputed.yearRange ?? state.availableFilters.yearRange,
-          variables: getVariables(filteredTables),
-          status: recomputed.status ?? state.availableFilters.status,
-        },
-        lastUsedYearRange: updatedLastUsedYearRange,
-      };
-    }
+    case ActionType.REMOVE_FILTERS:
+      return applyRemoveFilters(state, action.payload);
 
     case ActionType.SET_ERROR:
       return { ...state, error: action.payload };
@@ -343,3 +291,100 @@ function reducer(
       return state;
   }
 }
+
+type RemovePayload = { value: string; type: FilterType; uniqueId?: string };
+
+// Removes one or many filters and recomputes derived table/filter state.
+// Subject filters can be removed by uniqueId to avoid collisions where multiple
+// nodes share the same subject id value.
+function applyRemoveFilters(
+  state: StartPageState,
+  removals: RemovePayload[],
+): StartPageState {
+  // Precise removal key for subject tree nodes.
+  const removalSetByUniqueId = new Set(
+    removals
+      .filter((r) => r.type === 'subject' && r.uniqueId)
+      .map((r) => r.uniqueId as string),
+  );
+  // Generic key for non-subject filters, and subject fallback by type+value.
+  const removalSetByTypeValue = new Set(
+    removals.map((r) => `${r.type}|${r.value}`),
+  );
+
+  const currentFilters = state.activeFilters.filter((f) => {
+    if (f.type === 'subject' && f.uniqueId && removalSetByUniqueId.size > 0) {
+      if (removalSetByUniqueId.has(f.uniqueId)) {
+        return false;
+      }
+    }
+    return !removalSetByTypeValue.has(`${f.type}|${f.value}`);
+  });
+
+  if (currentFilters.length === 0) {
+    const fullRange = getYearRanges(state.availableTables);
+    return {
+      ...state,
+      availableTablesWhenQueryApplied: [],
+      activeFilters: [],
+      filteredTables: state.availableTables,
+      availableFilters: {
+        subjectTree: updateSubjectTreeCounts(
+          state.originalSubjectTree,
+          state.availableTables,
+        ),
+        timeUnits: getTimeUnits(state.availableTables),
+        yearRange: fullRange,
+        variables: getVariables(state.availableTables),
+        status: getStatus(state.availableTables),
+      },
+      lastUsedYearRange: fullRange,
+    };
+  }
+
+  // If query was removed, filtering must resume from the full dataset.
+  // Otherwise, keep the currently query-scoped base table set.
+  const removedQuery = removals.some((r) => r.type === 'query');
+  const baseTables = removedQuery
+    ? state.availableTables
+    : getAvailableTables(state);
+  const matcher = buildCompiledMatcher(currentFilters);
+
+  const filteredTables = baseTables.filter((table) =>
+    shouldTableBeIncludedWithMatcher(table, matcher),
+  );
+
+  const yearRangeStillActive = currentFilters.some(
+    (f) => f.type === 'yearRange',
+  );
+  // Preserve previously remembered range while a year filter is still active.
+  const updatedLastUsedYearRange = yearRangeStillActive
+    ? state.lastUsedYearRange
+    : getYearRanges(filteredTables);
+
+  const removedTypeHint = removals[0]?.type as FilterType | undefined;
+
+  const recomputed = recomputeAvailableFilters(
+    removedTypeHint,
+    currentFilters,
+    baseTables,
+    state.originalSubjectTree,
+  );
+
+  return {
+    ...state,
+    availableTablesWhenQueryApplied: removedQuery
+      ? []
+      : state.availableTablesWhenQueryApplied,
+    activeFilters: currentFilters,
+    filteredTables,
+    availableFilters: {
+      subjectTree: recomputed.subjectTree ?? state.availableFilters.subjectTree,
+      timeUnits: recomputed.timeUnits ?? state.availableFilters.timeUnits,
+      yearRange: recomputed.yearRange ?? state.availableFilters.yearRange,
+      variables: getVariables(filteredTables),
+      status: recomputed.status ?? state.availableFilters.status,
+    },
+    lastUsedYearRange: updatedLastUsedYearRange,
+  };
+}

packages/pxweb2/src/app/pages/StartPage/StartPageTypes.ts

@@ -10,6 +10,7 @@ export enum ActionType {
   ADD_SEARCH_FILTER = 'ADD_SEARCH_FILTER',
   ADD_QUERY_FILTER = 'ADD_QUERY_FILTER',
   REMOVE_FILTER = 'REMOVE_FILTER',
+  REMOVE_FILTERS = 'REMOVE_FILTERS',
   UPDATE_TABLES = 'UPDATE_TABLES',
   SET_ERROR = 'SET_ERROR',
   SET_LOADING = 'SET_LOADING',
@@ -67,13 +68,19 @@ export type ReducerActionTypes =
   | RemoveFilterAction
   | UpdateTablesAction
   | SetErrorAction
-  | SetLoadingAction;
+  | SetLoadingAction
+  | RemoveFiltersAction;
 
 type RemoveFilterAction = {
   type: ActionType.REMOVE_FILTER;
   payload: { value: string; type: FilterType; uniqueId?: string };
 };
 
+type RemoveFiltersAction = {
+  type: ActionType.REMOVE_FILTERS;
+  payload: Array<{ value: string; type: FilterType; uniqueId?: string }>;
+};
+
 type ResetFilterAction = {
   type: ActionType.RESET_FILTERS;
   payload: { tables: Table[]; subjects: PathItem[] };