refactor: separate SimpleFilter values using commas (#75)

Author: bitnimble

.claude/settings.local.json

@@ -39,9 +39,7 @@
       "Bash(git rm *)",
       "Bash(git show *)"
     ],
-    "deny": [
-      "Write(supabase/migrations/*)"
-    ],
+    "deny": ["Write(supabase/migrations/*)"],
     "defaultMode": "acceptEdits"
   }
 }

src/app/filter_builder.tsx

@@ -4,10 +4,10 @@ import { MapListStore } from 'app/map_list_presenter';
 import {
   SIMPLE_FIELDS,
   SimpleField,
-  getFieldValue,
+  compileSimpleFilter,
+  filterToSimpleValues,
   isSimpleFilter,
-  setFieldValue,
-  toSimpleFilter,
+  simpleFieldKey,
 } from 'app/filter_modes';
 import { X } from 'lucide-react';
 import { action } from 'mobx';
@@ -56,8 +56,6 @@ const FIELD_LABELS: Record<FilterableField, string> = {
 // dedicated UI land (next PR); today the column is never populated, so the field would match nothing.
 const ALL_FIELDS = filterableFields.filter((f) => f !== 'tags');
 
-const simpleFieldKey = (simpleField: SimpleField) => `${simpleField.field}:${simpleField.op}`;
-
 // Derive each simple field's label from the shared field/op labels so they can't drift; date slots
 // read as "Uploaded before/after" rather than the bare field label.
 const simpleFieldLabel = (simpleField: SimpleField) =>
@@ -70,12 +68,16 @@ const simpleInputType = (simpleField: SimpleField) => {
   if (kind === 'date') {
     return 'date';
   }
-  if (kind === 'number' || kind === 'countable') {
+  if (kind === 'number') {
     return 'number';
   }
+  // `countable` (Difficulties) stays a text input so comma-separated values like "1, 2" can be typed.
   return 'text';
 };
 
+const simpleFieldPlaceholder = (simpleField: SimpleField) =>
+  simpleField.field === 'difficulties' ? 'e.g. 1, 2' : undefined;
+
 export const FilterBuilder = observer((props: { store: MapListStore; onSearch: () => void }) => {
   const { store, onSearch } = props;
   return (
@@ -118,7 +120,7 @@ export const ActiveFilterPills = observer(
 
     const active = SIMPLE_FIELDS.map((simpleField) => ({
       simpleField,
-      value: getFieldValue(store.filter, simpleField),
+      value: store.simpleValues.get(simpleFieldKey(simpleField)) ?? '',
     })).filter(({ value }) => value.trim() !== '');
     if (active.length === 0) {
       return null;
@@ -130,7 +132,7 @@ export const ActiveFilterPills = observer(
             key={simpleFieldKey(simpleField)}
             label={`${simpleFieldLabel(simpleField)}: ${value}`}
             onRemove={action(() => {
-              store.filter = setFieldValue(store.filter, simpleField, '');
+              store.simpleValues.delete(simpleFieldKey(simpleField));
               onSearch();
             })}
           />
@@ -156,8 +158,10 @@ const Pill = (props: { label: string; onRemove: () => void }) => (
 
 const SimpleBuilder = observer((props: { store: MapListStore; onSearch: () => void }) => {
   const { store, onSearch } = props;
+  // Unidirectional: the textboxes own their raw strings in `simpleValues`; only user input writes
+  // them, and they're compiled to the AST only on search (see `MapListStore.activeFilter`).
   const setField = action((simpleField: SimpleField, value: string) => {
-    store.filter = setFieldValue(store.filter, simpleField, value);
+    store.simpleValues.set(simpleFieldKey(simpleField), value);
   });
   return (
     <div className={styles.simple}>
@@ -167,7 +171,8 @@ const SimpleBuilder = observer((props: { store: MapListStore; onSearch: () => vo
           label={simpleFieldLabel(simpleField)}
           error={undefined}
           inputType={simpleInputType(simpleField)}
-          value={getFieldValue(store.filter, simpleField)}
+          placeholder={simpleFieldPlaceholder(simpleField)}
+          value={store.simpleValues.get(simpleFieldKey(simpleField)) ?? ''}
           onChange={(v) => setField(simpleField, v)}
           onSubmit={onSearch}
         />
@@ -398,22 +403,23 @@ const ToggleButton = (props: {
 
 const ModeSwitch = observer((props: { store: MapListStore }) => {
   const { store } = props;
-  // Simple and advanced edit the same AST, so switching to advanced needs no conversion.
   const toAdvanced = action(() => {
+    // Hand the compiled simple filter to advanced mode so the AST it edits matches what was active.
+    store.filter = compileSimpleFilter(store.simpleValues);
     store.filterMode = 'advanced';
   });
   const toSimple = action(() => {
-    if (isSimpleFilter(store.filter)) {
-      store.filterMode = 'simple';
-      return;
-    }
-    const ok = window.confirm(
-      'Switching to simple filters will discard the parts of your filter that simple mode cannot represent. Continue?'
-    );
-    if (ok) {
-      store.filter = toSimpleFilter(store.filter);
-      store.filterMode = 'simple';
+    if (!isSimpleFilter(store.filter)) {
+      const ok = window.confirm(
+        'Switching to simple filters will discard the parts of your filter that simple mode cannot represent. Continue?'
+      );
+      if (!ok) {
+        return;
+      }
     }
+    store.simpleValues = filterToSimpleValues(store.filter);
+    store.filter = undefined;
+    store.filterMode = 'simple';
   });
   return (
     <button

src/app/filter_modes.ts

@@ -1,132 +1,165 @@
 import { CmpNode, FILTER_FIELDS, FilterNode, FilterOp, FilterableField } from 'schema/map_filter';
 
 /**
- * Simple and advanced filtering differ only in the UI: both edit the same {@link FilterNode}. Simple
- * mode is a constrained editor over a flat `and` of cmps with pre-selected fields, each
- * {@link SimpleField} fixes a `(field, op)` pair so the user only supplies a value. The helpers here
- * read and write those fields on the shared AST.
+ * Simple and advanced filtering differ in their edit model. Advanced mode edits the shared
+ * {@link FilterNode} AST directly. Simple mode is a constrained editor: each {@link SimpleField}
+ * fixes a `(field, op)` pair and owns a raw string the user types, compiled to the AST only on
+ * search ({@link compileSimpleFilter}). A `multi` field treats its value as a comma-separated list
+ * of independent terms OR'd together (e.g. mapper `Foo,Bar`, difficulties `1,2`); comma support is a
+ * property of the field, not the operator (so `description`, also `contains`, stays single-valued).
  */
-export type SimpleField = { field: FilterableField; op: FilterOp };
+export type SimpleField = { field: FilterableField; op: FilterOp; multi?: boolean };
 
 export const SIMPLE_FIELDS: SimpleField[] = [
-  { field: 'artist', op: 'contains' },
-  { field: 'author', op: 'contains' },
+  { field: 'artist', op: 'contains', multi: true },
+  { field: 'author', op: 'contains', multi: true },
   { field: 'description', op: 'contains' },
-  { field: 'difficulties', op: 'count' },
+  { field: 'difficulties', op: 'count', multi: true },
   { field: 'submissionDate', op: 'after' },
   { field: 'submissionDate', op: 'before' },
 ];
 
-const matches = (node: CmpNode, simpleField: SimpleField) =>
-  node.field === simpleField.field && node.op === simpleField.op;
+export const simpleFieldKey = (sf: SimpleField) => `${sf.field}:${sf.op}`;
+
+const matches = (node: CmpNode, sf: SimpleField) => node.field === sf.field && node.op === sf.op;
 
 const simpleFieldOf = (node: CmpNode) => SIMPLE_FIELDS.find((sf) => matches(node, sf));
 
-// The cmp children of a simple-shaped filter (a flat `and`, a bare cmp, or empty/undefined).
-function simpleCmps(filter: FilterNode | undefined): CmpNode[] {
-  if (filter == null) {
-    return [];
+// A simple "slot" is the node holding one field's value: a bare cmp, or - for a `multi` field whose
+// value lists several comma terms - an `or` of cmps that all share the slot's (field, op).
+function slotCmps(node: FilterNode): CmpNode[] | undefined {
+  if (node.type === 'cmp') {
+    return [node];
+  }
+  if (
+    node.type === 'or' &&
+    node.children.length > 0 &&
+    node.children.every((c) => c.type === 'cmp')
+  ) {
+    return node.children as CmpNode[];
+  }
+  return undefined;
+}
+
+// The simple field a slot represents, iff all its cmps share one simple (field, op). An `or` slot is
+// only valid for a `multi` field, since single-valued fields never compile to an OR.
+function slotSimpleField(node: FilterNode): SimpleField | undefined {
+  const cmps = slotCmps(node);
+  if (cmps == null) {
+    return undefined;
   }
-  if (filter.type === 'cmp') {
-    return [filter];
+  const sf = simpleFieldOf(cmps[0]);
+  if (sf == null || !cmps.every((c) => matches(c, sf))) {
+    return undefined;
   }
-  if (filter.type === 'and') {
-    return filter.children.filter((c): c is CmpNode => c.type === 'cmp');
+  if (node.type === 'or' && !sf.multi) {
+    return undefined;
   }
-  return [];
+  return sf;
+}
+
+// The top-level slots of a simple-shaped filter (a flat `and`, a bare slot, or empty/undefined).
+function simpleSlots(filter: FilterNode | undefined): FilterNode[] {
+  if (filter == null) {
+    return [];
+  }
+  return filter.type === 'and' ? filter.children : [filter];
 }
 
 /**
- * True if the node fits simple mode: empty, a bare cmp, or a flat `and` whose children are all
- * distinct simple fields. Anything else (OR/NOT/nesting, other fields/operators, duplicates) needs
- * advanced mode.
+ * True if the node fits simple mode: empty, or a flat `and` (or a bare slot) whose children are all
+ * distinct simple slots - a cmp, or an `or` of comma terms for a `multi` field. Anything else (NOT,
+ * nesting, other fields/operators, duplicate fields) needs advanced mode.
  */
 export function isSimpleFilter(node: FilterNode | undefined): boolean {
   if (node == null) {
     return true;
   }
-  if (node.type !== 'cmp' && node.type !== 'and') {
-    return false;
-  }
-  const children: FilterNode[] = node.type === 'cmp' ? [node] : node.children;
   const seen = new Set<SimpleField>();
-  for (const child of children) {
-    if (child.type !== 'cmp') {
-      return false;
-    }
-    const simpleField = simpleFieldOf(child);
-    if (simpleField == null || seen.has(simpleField)) {
+  for (const slot of simpleSlots(node)) {
+    const sf = slotSimpleField(slot);
+    if (sf == null || seen.has(sf)) {
       return false;
     }
-    seen.add(simpleField);
+    seen.add(sf);
   }
   return true;
 }
 
-export function getFieldValue(filter: FilterNode | undefined, simpleField: SimpleField): string {
-  const cmp = simpleCmps(filter).find((c) => matches(c, simpleField));
-  return cmp == null ? '' : String(cmp.value);
-}
-
 /**
- * Immutably sets (or, for a blank value, clears) a simple field on the filter, returning the rebuilt
- * flat `and`, or undefined when no fields remain. Other fields keep their position.
+ * Compiles the raw simple-field strings into the {@link FilterNode} AST sent to the backend, API and
+ * URL. Run only on search: each field splits on commas when `multi`, trims terms, drops empties,
+ * coerces numeric/countable terms (dropping non-numbers), and is omitted entirely when nothing valid
+ * remains. Several terms become an `or`, a single term a bare cmp. Returns undefined when empty, so
+ * an untouched or all-blank builder behaves like no filter.
  */
-export function setFieldValue(
-  filter: FilterNode | undefined,
-  simpleField: SimpleField,
-  value: string
-): FilterNode | undefined {
-  const children = [...simpleCmps(filter)];
-  const idx = children.findIndex((c) => matches(c, simpleField));
-  if (value.trim() === '') {
-    if (idx >= 0) {
-      children.splice(idx, 1);
-    }
-  } else {
-    // Numeric kinds carry a `number` in the AST; the simple-mode widget hands us its raw string, so
-    // coerce here (the field's blank state was already handled above).
-    const kind = FILTER_FIELDS[simpleField.field].kind;
-    const coerced = kind === 'number' || kind === 'countable' ? Number(value) : value;
-    const cmp: CmpNode = {
-      type: 'cmp',
-      field: simpleField.field,
-      op: simpleField.op,
-      value: coerced,
-    };
-    if (idx >= 0) {
-      children[idx] = cmp;
-    } else {
-      children.push(cmp);
+export function compileSimpleFilter(values: ReadonlyMap<string, string>): FilterNode | undefined {
+  const children: FilterNode[] = [];
+  for (const sf of SIMPLE_FIELDS) {
+    const node = compileSlot(sf, values.get(simpleFieldKey(sf)) ?? '');
+    if (node != null) {
+      children.push(node);
     }
   }
   return children.length === 0 ? undefined : { type: 'and', children };
 }
 
+function compileSlot(sf: SimpleField, raw: string): FilterNode | undefined {
+  const kind = FILTER_FIELDS[sf.field].kind;
+  const numeric = kind === 'number' || kind === 'countable';
+  const terms = sf.multi ? raw.split(',') : [raw];
+  const cmps: CmpNode[] = [];
+  for (const term of terms) {
+    const trimmed = term.trim();
+    if (trimmed === '') {
+      continue;
+    }
+    let value: string | number = trimmed;
+    if (numeric) {
+      const n = Number(trimmed);
+      // The difficulties widget is a text input, so it can hold non-numeric junk; drop those terms.
+      if (!Number.isFinite(n)) {
+        continue;
+      }
+      value = n;
+    }
+    cmps.push({ type: 'cmp', field: sf.field, op: sf.op, value });
+  }
+  if (cmps.length === 0) {
+    return undefined;
+  }
+  return cmps.length === 1 ? cmps[0] : { type: 'or', children: cmps };
+}
+
 /**
- * Best-effort reduction of an arbitrary tree to its simple-representable parts, used when the user
- * switches advanced → simple and accepts discarding the incompatible parts. Keeps the first cmp per
- * simple field; `not` subtrees are dropped since a negated clause has no simple form.
+ * Decompiles a filter back into raw simple-field strings, for URL rehydration and the
+ * advanced -> simple switch. Recognized slots become their field's value (joining a `multi` field's
+ * OR terms with commas); the first occurrence per field wins and anything not simple-representable is
+ * dropped, matching the switch's discard prompt.
  */
-export function toSimpleFilter(node: FilterNode | undefined): FilterNode | undefined {
-  const children: CmpNode[] = [];
-  const seen = new Set<SimpleField>();
+export function filterToSimpleValues(node: FilterNode | undefined): Map<string, string> {
+  const values = new Map<string, string>();
   const visit = (n: FilterNode) => {
-    if (n.type === 'cmp') {
-      const simpleField = simpleFieldOf(n);
-      if (simpleField != null && !seen.has(simpleField)) {
-        seen.add(simpleField);
-        children.push({ ...n });
+    const sf = slotSimpleField(n);
+    if (sf != null) {
+      const key = simpleFieldKey(sf);
+      if (!values.has(key)) {
+        values.set(
+          key,
+          slotCmps(n)!
+            .map((c) => String(c.value))
+            .join(',')
+        );
       }
       return;
     }
-    if (n.type === 'not') {
-      return;
+    // Not a recognized slot: salvage simple cmps from inside groups; drop cmps/NOT we can't represent.
+    if (n.type === 'and' || n.type === 'or') {
+      n.children.forEach(visit);
     }
-    n.children.forEach(visit);
   };
-  if (node) {
+  if (node != null) {
     visit(node);
   }
-  return children.length === 0 ? undefined : { type: 'and', children };
+  return values;
 }