fix(tags): commit option selections through GraphQL Soup

apps/web/src/features/property/tags/EntityRowTags.tsx

@@ -2,6 +2,7 @@ import CaretDownIcon from '@phosphor/caret-down.svg';
 import CircleDashedEmpty from '@phosphor/circle-dashed.svg';
 import FilterIcon from '@phosphor/funnel-simple.svg';
 import PencilIcon from '@phosphor/pencil-simple.svg';
+import { useInFlightEntityPropertyOptions } from '@queries/properties/in-flight-options';
 import { EntityType } from '@service-properties/generated/schemas/entityType';
 import type { SoupProperty } from '@service-storage/generated/schemas/soupProperty';
 import { Button, cn, HoverCard, Layer } from '@ui';
@@ -279,7 +280,10 @@ export function EntityRowTags(props: {
   class?: string;
   onFilterByTag?: (optionId: string) => void;
 }) {
-  const appliedTags = useSoupResolvedTags(() => props.properties);
+  const appliedTags = useSoupResolvedTags(
+    () => props.properties,
+    useInFlightEntityPropertyOptions(props.entityId)
+  );
   const createDocTags = () =>
     useSoupDocTags(props.entityId, props.entityType, () => props.properties);
   const maxVisible = () => props.maxVisible ?? DEFAULT_MAX_VISIBLE;

apps/web/src/features/property/tags/tag-sets-context.test.tsx

@@ -6,6 +6,7 @@ import { EntityType } from '@service-properties/generated/schemas/entityType';
 import type { TagSetResponse } from '@service-properties/generated/schemas/tagSetResponse';
 import type { SoupProperty } from '@service-storage/generated/schemas/soupProperty';
 import { fireEvent, render, screen } from '@solidjs/testing-library';
+import { QueryClient, QueryClientProvider } from '@tanstack/solid-query';
 import type { JSX } from 'solid-js';
 import { beforeEach, describe, expect, it, vi } from 'vitest';
 import { EntityRowTags } from './EntityRowTags';
@@ -65,14 +66,18 @@ describe('TagSetsContext', () => {
       } as SoupProperty,
     ];
 
+    // Rows read pending option mutations to overlay an uncommitted selection,
+    // so they need a client even though they start no mutation of their own.
     render(() => (
-      <TagSetsProvider tagSets={tagSets}>
-        <EntityRowTags
-          entityId="document-1"
-          entityType={EntityType.DOCUMENT}
-          properties={properties}
-        />
-      </TagSetsProvider>
+      <QueryClientProvider client={new QueryClient()}>
+        <TagSetsProvider tagSets={tagSets}>
+          <EntityRowTags
+            entityId="document-1"
+            entityType={EntityType.DOCUMENT}
+            properties={properties}
+          />
+        </TagSetsProvider>
+      </QueryClientProvider>
     ));
 
     expect(screen.getByText('Urgent')).toBeTruthy();

apps/web/src/features/property/tags/useDocTags.ts

@@ -1,7 +1,5 @@
-import {
-  useBulkUpdateEntityPropertyOptionsMutation,
-  useInFlightEntityPropertyOptions,
-} from '@queries/properties/entity';
+import { useBulkUpdateEntityPropertyOptionsMutation } from '@queries/properties/entity';
+import { useInFlightEntityPropertyOptions } from '@queries/properties/in-flight-options';
 import {
   useEnsureTagSetMutation,
   useTagsQuery,
@@ -84,8 +82,9 @@ function createDocTags(
   appliedOptionIdsForDefinition: (definitionId: string) => string[],
   persistTagSelection: PersistTagSelection,
   tagSets: Accessor<TagSetResponse[]>,
-  // Optimistic overlay for query-backed sources (undefined for soup/local,
-  // whose sources update optimistically on their own).
+  // Optimistic overlay for sources a mutation cannot write through (query
+  // results, and soup rows whose property record does not exist yet).
+  // Undefined for local sources, which are set synchronously.
   inFlightOptionIdsForDefinition?: (
     definitionId: string
   ) => string[] | undefined
@@ -295,7 +294,12 @@ export function useDocTags(entityId: string, entityType: EntityType) {
 /**
  * Doc-tags backed by an entity's already-loaded soup properties instead of a
  * per-entity fetch. List rows use this so tags render with no extra requests.
- * Mutations patch the soup cache optimistically, so the source stays live.
+ *
+ * Mutations patch the soup cache optimistically, but only where the entity has
+ * a property record to patch: the first tag from a set has no assignment id
+ * until the server answers. The in-flight overlay covers that gap (and any
+ * transport whose cache this row does not read from), so a picked tag always
+ * shows immediately.
  */
 export function useSoupDocTags(
   entityId: string,
@@ -311,11 +315,14 @@ export function useSoupDocTags(
   };
   const persistTagSelection = usePersistTagSelection(entityId, entityType);
   const tagSets = useTagSets();
+  const inFlightOptionIdsForDefinition =
+    useInFlightEntityPropertyOptions(entityId);
 
   return createDocTags(
     appliedOptionIdsForDefinition,
     persistTagSelection,
-    tagSets
+    tagSets,
+    inFlightOptionIdsForDefinition
   );
 }
 

apps/web/src/features/property/tags/useSoupResolvedTags.ts

@@ -20,9 +20,18 @@ function optionLabel(option: PropertyOptionResponse): string {
  * Resolves the tags already present in soup properties without initializing
  * any edit mutations. Virtual rows use this read-only model until a picker is
  * actually opened.
+ *
+ * `inFlightOptionIdsForDefinition` overlays an uncommitted selection. Applying
+ * an entity's first tag from a set creates a property record whose id only
+ * exists once the server answers, so that write cannot be reflected in the
+ * cache beforehand — without the overlay the chip would appear a round trip
+ * late.
  */
 export function useSoupResolvedTags(
-  properties: Accessor<SoupProperty[] | undefined>
+  properties: Accessor<SoupProperty[] | undefined>,
+  inFlightOptionIdsForDefinition?: (
+    definitionId: string
+  ) => string[] | undefined
 ): Accessor<ResolvedTag[]> {
   const tagSets = useTagSets();
   const tagOptionById = useTagOptionById();
@@ -39,9 +48,12 @@ export function useSoupResolvedTags(
         (candidate) => candidate.definition.id === definitionId
       );
       const value = property?.value;
-      if (value?.type !== 'SelectOption') continue;
+      const optionIds =
+        inFlightOptionIdsForDefinition?.(definitionId) ??
+        (value?.type === 'SelectOption' ? value.value : undefined);
+      if (!optionIds) continue;
 
-      for (const optionId of value.value) {
+      for (const optionId of optionIds) {
         const tagOption = options.get(optionId);
         if (!tagOption) continue;
         resolved.push({

apps/web/src/lib/queries/properties/entity.test.tsx

@@ -33,6 +33,7 @@ const getRestEntityPropertiesMock = vi.hoisted(() => vi.fn());
 const deleteEntityPropertyMock = vi.hoisted(() => vi.fn());
 const addEntityPropertyOptionMock = vi.hoisted(() => vi.fn());
 const bulkUpdateEntityPropertyOptionsMock = vi.hoisted(() => vi.fn());
+const updateGraphqlEntityPropertyOptionsMock = vi.hoisted(() => vi.fn());
 const setRestEntityPropertyMock = vi.hoisted(() => vi.fn());
 const isInstantiatedPropertyMock = vi.hoisted(() => vi.fn());
 const entityPropertyFromApiMock = vi.hoisted(() => vi.fn());
@@ -93,6 +94,10 @@ vi.mock('./graphql/entity', () => ({
     createGraphqlBulkSaveEntityPropertiesMutationMock,
 }));
 
+vi.mock('./graphql/entity-options', () => ({
+  updateGraphqlEntityPropertyOptions: updateGraphqlEntityPropertyOptionsMock,
+}));
+
 vi.mock('../client', () => ({
   get queryClient() {
     return testQueryClient;
@@ -775,6 +780,9 @@ describe('useBulkSaveEntityPropertiesMutation dispositions', () => {
   });
 
   it('invalidates REST after failed REST-backed writes', async () => {
+    // Option selections have their own GraphQL transport, so this REST-only
+    // assertion needs the flag off for the bulk-options write below.
+    graphqlSoupEnabledMock.mockReturnValue(false);
     deleteEntityPropertyMock.mockResolvedValue(
       err([{ code: 'SERVER_ERROR', message: 'delete failed' }])
     );
@@ -835,4 +843,55 @@ describe('useBulkSaveEntityPropertiesMutation dispositions', () => {
     expect(testQueryClient.invalidateQueries).toHaveBeenCalled();
     expect(toastFailureMock).toHaveBeenCalledOnce();
   });
+
+  const optionSelection = {
+    entityId: 'task-1',
+    entityType: 'TASK' as const,
+    properties: [
+      { property, currentOptionIds: ['todo'], nextOptionIds: ['doing'] },
+    ],
+  };
+
+  it('commits option selections through GraphQL when Soup is GraphQL-backed', async () => {
+    updateGraphqlEntityPropertyOptionsMock.mockResolvedValue([
+      { propertyDefinitionId: 'status-def', optionIds: ['doing'] },
+    ]);
+
+    await expect(
+      bulkOptionsMutation.mutateAsync(optionSelection)
+    ).resolves.toEqual([
+      { propertyDefinitionId: 'status-def', optionIds: ['doing'] },
+    ]);
+
+    expect(updateGraphqlEntityPropertyOptionsMock).toHaveBeenCalledWith(
+      optionSelection
+    );
+    expect(bulkUpdateEntityPropertyOptionsMock).not.toHaveBeenCalled();
+  });
+
+  it('rolls back and reports a failed GraphQL option selection', async () => {
+    updateGraphqlEntityPropertyOptionsMock.mockRejectedValue(
+      new Error('options failed')
+    );
+
+    await expect(
+      bulkOptionsMutation.mutateAsync(optionSelection)
+    ).rejects.toThrow('options failed');
+
+    expect(bulkUpdateEntityPropertyOptionsMock).not.toHaveBeenCalled();
+    expect(toastFailureMock).toHaveBeenCalledOnce();
+  });
+
+  it('keeps option selections on REST when GraphQL Soup is disabled', async () => {
+    graphqlSoupEnabledMock.mockReturnValue(false);
+
+    await expect(
+      bulkOptionsMutation.mutateAsync(optionSelection)
+    ).resolves.toEqual([
+      { propertyDefinitionId: 'status-def', optionIds: ['doing'] },
+    ]);
+
+    expect(bulkUpdateEntityPropertyOptionsMock).toHaveBeenCalledOnce();
+    expect(updateGraphqlEntityPropertyOptionsMock).not.toHaveBeenCalled();
+  });
 });

apps/web/src/lib/queries/properties/entity.ts

@@ -18,7 +18,7 @@ import type {
   PropertyDefinitionDomain,
 } from '@property/types';
 import { isInstantiatedProperty } from '@property/utils';
-import { useMutation, useMutationState, useQuery } from '@tanstack/solid-query';
+import { useMutation, useQuery } from '@tanstack/solid-query';
 import { type Accessor, batch } from 'solid-js';
 import { propertiesServiceClient } from '../../service-clients/service-properties/client';
 import type { EntityType } from '../../service-clients/service-properties/generated/schemas/entityType';
@@ -41,7 +41,16 @@ import {
   createGraphqlEntityPropertiesQuery,
   type EntityPropertyMutationDisposition,
 } from './graphql/entity';
+import { updateGraphqlEntityPropertyOptions } from './graphql/entity-options';
+import {
+  type BulkUpdateEntityPropertyOptionsParams,
+  bulkEntityPropertyOptionsKey,
+} from './in-flight-options';
 import { propertiesKeys } from './keys';
+import {
+  type EntityPropertyOptionSelection,
+  getEntityPropertyOptionDeltas,
+} from './option-deltas';
 
 function toPropertyTargetEntityType(
   entityType: EntityType | PropertyTargetEntityType
@@ -188,10 +197,21 @@ function optimisticUpdateSoupEntityProperties(
   }[]
 ): SoupTransaction | undefined {
   const current = getSoupEntityById(entityId);
+  if (!current) {
+    // A miss here means the write silently lost its optimism. Expected on the
+    // GraphQL transport, whose rows live in the normalized cache instead and
+    // carry their own optimistic write.
+    if (!ENABLE_GRAPHQL_SOUP()) {
+      console.warn(
+        'no soup cache entry for entity; skipping optimistic property update',
+        entityId
+      );
+    }
+    return undefined;
+  }
   // channel / foreign entity / channel thread rows are property-less; call
   // records carry properties (tags) and are handled like documents.
   if (
-    !current ||
     current.tag === 'channel' ||
     current.tag === 'foreignEntity' ||
     current.tag === 'channelThread' ||
@@ -591,91 +611,10 @@ export function useRemoveEntityPropertyOptionMutation(
   }));
 }
 
-type EntityPropertyOptionDelta = {
-  type: 'add' | 'remove';
-  optionId: string;
-};
-
-function getEntityPropertyOptionDeltas(
-  currentOptionIds: string[],
-  nextOptionIds: string[]
-): EntityPropertyOptionDelta[] {
-  const current = new Set(currentOptionIds);
-  const next = new Set(nextOptionIds);
-  return [
-    ...currentOptionIds
-      .filter((optionId) => !next.has(optionId))
-      .map((optionId) => ({ type: 'remove' as const, optionId })),
-    ...nextOptionIds
-      .filter((optionId) => !current.has(optionId))
-      .map((optionId) => ({ type: 'add' as const, optionId })),
-  ];
-}
-
-type BulkUpdateEntityPropertyOptionsParams = {
-  entityId: string;
-  entityType: EntityType;
-  properties: Array<{
-    property: Property | PropertyDefinitionDomain;
-    currentOptionIds: string[];
-    nextOptionIds: string[];
-  }>;
-};
-
-/** A property's reconciled final option ids after a bulk update. */
-export type EntityPropertyOptionSelection = {
-  propertyDefinitionId: string;
-  optionIds: string[];
-};
-
 type BulkUpdateEntityPropertyOptionsContext = {
   soupTxn?: SoupTransaction;
 };
 
-/**
- * Mutation-cache key for an entity's bulk option updates. Used both as the
- * mutation's serialization scope and to read its in-flight variables for
- * optimistic display.
- */
-function bulkEntityPropertyOptionsKey(entityId: string) {
-  return ['bulkEntityPropertyOptions', entityId] as const;
-}
-
-/**
- * Optimistic overlay for a query-backed tag source: the option ids an in-flight
- * bulk update is applying to a property, so a query-backed view reflects the
- * change before its refetch lands. Returns `undefined` when nothing is in
- * flight for the property, so callers fall back to the persisted value. On
- * settle the mutation leaves `pending` and the overlay disappears — no manual
- * rollback. Soup-backed sources don't need this: their optimism rides the
- * soup-cache update in the mutation lifecycle below.
- */
-export function useInFlightEntityPropertyOptions(entityId: string) {
-  const inFlight = useMutationState(() => ({
-    filters: {
-      mutationKey: bulkEntityPropertyOptionsKey(entityId),
-      status: 'pending' as const,
-    },
-    select: (mutation) =>
-      mutation.state.variables as
-        | BulkUpdateEntityPropertyOptionsParams
-        | undefined,
-  }));
-
-  return (propertyDefinitionId: string): string[] | undefined => {
-    const pending = inFlight();
-    // Latest in-flight update targeting this property wins.
-    for (let index = pending.length - 1; index >= 0; index--) {
-      const match = pending[index]?.properties.find(
-        (update) =>
-          getPropertyDefinitionId(update.property) === propertyDefinitionId
-      );
-      if (match) return match.nextOptionIds;
-    }
-    return undefined;
-  };
-}
-
 /**
  * Persists a complete multi-select selection across one or more properties in a
  * single transactional request, then reconciles the soup cache from the final
@@ -700,6 +639,12 @@ export function useBulkUpdateEntityPropertyOptionsMutation(
     mutationFn: async (
       variables: BulkUpdateEntityPropertyOptionsParams
     ): Promise<EntityPropertyOptionSelection[]> => {
+      // The transport swaps, the mutation shell does not: the per-entity scope
+      // that serializes commits and the in-flight overlay both read this
+      // mutation's state, whichever cache the selection lands in.
+      if (ENABLE_GRAPHQL_SOUP()) {
+        return updateGraphqlEntityPropertyOptions(variables);
+      }
       const response = await throwOnErr(async () =>
         propertiesServiceClient.bulkUpdateEntityPropertyOptions({
           entity_type: toPropertyTargetEntityType(variables.entityType),
@@ -712,12 +657,8 @@ export function useBulkUpdateEntityPropertyOptionsMutation(
               );
               return {
                 property_id: getPropertyDefinitionId(update.property),
-                add_option_ids: deltas
-                  .filter((delta) => delta.type === 'add')
-                  .map((delta) => delta.optionId),
-                remove_option_ids: deltas
-                  .filter((delta) => delta.type === 'remove')
-                  .map((delta) => delta.optionId),
+                add_option_ids: deltas.addOptionIds,
+                remove_option_ids: deltas.removeOptionIds,
               };
             }),
           },