fix(ui-react): save the Models step by diff, not delete-then-insert (#107)

ui-react/src/graphql/generated/modeling.ts

@@ -1239,17 +1239,32 @@ export function generateModelingId(_type: 'problem_statement' | 'task' | 'thread
 // ─── Mutation: SetThreadModels ────────────────────────────────────────────────
 
 /**
- * Replace all model selections for a thread in a single transaction.
- * Deletes existing thread_model rows then inserts the new selection.
+ * Apply a change of model selection for a thread in a single transaction.
+ *
+ * `$removedIds` are the `thread_model.id` values that are no longer selected.
+ * Rows that stay selected are not named here, so they keep their id and the
+ * dataset and parameter bindings that hang off it.
+ *
+ * The four child tables are deleted first, scoped to the removed rows only:
+ * every one of them references `thread_model.id` with `ON DELETE RESTRICT`, so
+ * without this the delete is refused for any thread that has been through the
+ * Datasets or Parameters step (monorepo#107). Hasura runs a mutation's root
+ * fields in order inside one transaction, so the ordering here is the ordering
+ * Postgres sees.
  */
 export type SetThreadModelsMutationVariables = {
   threadId: string;
+  removedIds: string[];
   models: Array<{ thread_id: string; modelcatalog_configuration_id: string }>;
   userid: string;
   notes?: string | null;
 };
 
 export type SetThreadModelsMutation = {
+  delete_thread_model_execution_summary?: { affected_rows: number } | null;
+  delete_thread_model_execution?: { affected_rows: number } | null;
+  delete_thread_model_io?: { affected_rows: number } | null;
+  delete_thread_model_parameter?: { affected_rows: number } | null;
   delete_thread_model?: { affected_rows: number } | null;
   insert_thread_model?: {
     returning: Array<{ id: string; thread_id: string; modelcatalog_configuration_id?: string | null }>;
@@ -1260,11 +1275,26 @@ export type SetThreadModelsMutation = {
 export const SetThreadModelsDocument = gql`
   mutation SetThreadModels(
     $threadId: String!
+    $removedIds: [uuid!]!
     $models: [thread_model_insert_input!]!
     $userid: String!
     $notes: String
   ) {
-    delete_thread_model(where: { thread_id: { _eq: $threadId } }) {
+    delete_thread_model_execution_summary(
+      where: { thread_model_id: { _in: $removedIds } }
+    ) {
+      affected_rows
+    }
+    delete_thread_model_execution(where: { thread_model_id: { _in: $removedIds } }) {
+      affected_rows
+    }
+    delete_thread_model_io(where: { thread_model_id: { _in: $removedIds } }) {
+      affected_rows
+    }
+    delete_thread_model_parameter(where: { thread_model_id: { _in: $removedIds } }) {
+      affected_rows
+    }
+    delete_thread_model(where: { id: { _in: $removedIds } }) {
       affected_rows
     }
     insert_thread_model(objects: $models) {

ui-react/src/lib/__tests__/thread-models.test.ts

@@ -0,0 +1,59 @@
+import { describe, expect, it } from 'vitest';
+import { diffThreadModels } from '../thread-models';
+
+const row = (id: string, configId: string | null) => ({
+  id,
+  modelcatalog_configuration_id: configId,
+});
+
+describe('diffThreadModels', () => {
+  it('reports no change when the selection matches what is stored', () => {
+    const changes = diffThreadModels(
+      't1',
+      [row('tm-a', 'cfgA'), row('tm-b', 'cfgB')],
+      ['cfgB', 'cfgA'],
+    );
+    expect(changes).toEqual({ removedIds: [], added: [], unchanged: true });
+  });
+
+  it('keeps a still-selected row instead of deleting and re-inserting it', () => {
+    const changes = diffThreadModels('t1', [row('tm-a', 'cfgA')], ['cfgA', 'cfgB']);
+    expect(changes.removedIds).toEqual([]);
+    expect(changes.added).toEqual([{ thread_id: 't1', modelcatalog_configuration_id: 'cfgB' }]);
+    expect(changes.unchanged).toBe(false);
+  });
+
+  it('removes only the rows whose configuration was deselected', () => {
+    const changes = diffThreadModels('t1', [row('tm-a', 'cfgA'), row('tm-b', 'cfgB')], ['cfgA']);
+    expect(changes.removedIds).toEqual(['tm-b']);
+    expect(changes.added).toEqual([]);
+  });
+
+  it('leaves a row with no configuration id alone', () => {
+    // 21 of TACC's 109 thread_model rows are in this state. The step cannot
+    // show them, and deleting one would hit the same ON DELETE RESTRICT wall
+    // this diff exists to avoid.
+    const changes = diffThreadModels('t1', [row('tm-legacy', null)], ['cfgA']);
+    expect(changes.removedIds).toEqual([]);
+    expect(changes.added).toEqual([{ thread_id: 't1', modelcatalog_configuration_id: 'cfgA' }]);
+  });
+
+  it('reports no change for a thread that holds only unselectable legacy rows', () => {
+    expect(diffThreadModels('t1', [row('tm-legacy', null)], []).unchanged).toBe(true);
+  });
+
+  it('inserts every selection for a thread that holds no rows yet', () => {
+    const changes = diffThreadModels('t1', [], ['cfgA', 'cfgB']);
+    expect(changes.added).toEqual([
+      { thread_id: 't1', modelcatalog_configuration_id: 'cfgA' },
+      { thread_id: 't1', modelcatalog_configuration_id: 'cfgB' },
+    ]);
+    expect(changes.removedIds).toEqual([]);
+  });
+
+  it('does not re-insert a configuration that is stored twice', () => {
+    const changes = diffThreadModels('t1', [row('tm-a', 'cfgA'), row('tm-a2', 'cfgA')], ['cfgA']);
+    expect(changes.added).toEqual([]);
+    expect(changes.removedIds).toEqual([]);
+  });
+});

ui-react/src/lib/thread-models.ts

@@ -0,0 +1,71 @@
+/**
+ * Diffing the Models step's selection against the `thread_model` rows a thread
+ * already holds.
+ *
+ * The Models step used to save by deleting every `thread_model` row for the
+ * thread and re-inserting the selection. Four tables reference
+ * `thread_model.id` with `ON DELETE RESTRICT` — `thread_model_execution_summary`,
+ * `thread_model_execution`, `thread_model_io` and `thread_model_parameter` — so
+ * once the thread had been through the Datasets or Parameters step, the delete
+ * was refused and the step became a dead end (monorepo#107).
+ *
+ * The fix is to touch only what changed: keep the rows whose configuration is
+ * still selected, delete the rows whose configuration is not, insert the rows
+ * that are new. Keeping a row keeps its id, so the dataset and parameter
+ * bindings hanging off it survive a trip back to the Models step.
+ *
+ * A row with no `modelcatalog_configuration_id` is left alone. The step cannot
+ * display or select such a row, `threadModelFromGQL` skips it, and deleting it
+ * would hit the same RESTRICT wall on any legacy thread that holds runs.
+ * TACC's database holds 21 of them across 109 `thread_model` rows.
+ */
+
+/** The `thread_model` fields this module needs; `GetThread` selects both. */
+export interface ExistingThreadModel {
+  id: string;
+  modelcatalog_configuration_id?: string | null;
+}
+
+export interface ThreadModelInsert {
+  thread_id: string;
+  modelcatalog_configuration_id: string;
+}
+
+export interface ThreadModelChanges {
+  /** `thread_model.id` values to delete, with their bindings and runs. */
+  removedIds: string[];
+  /** Rows to insert for newly selected configurations. */
+  added: ThreadModelInsert[];
+  /** True when the selection matches what is stored, so no write is needed. */
+  unchanged: boolean;
+}
+
+export function diffThreadModels(
+  threadId: string,
+  existing: readonly ExistingThreadModel[],
+  selectedConfigIds: Iterable<string>,
+): ThreadModelChanges {
+  const selected = new Set(selectedConfigIds);
+  const removedIds: string[] = [];
+  const stored = new Set<string>();
+
+  for (const row of existing) {
+    const configId = row.modelcatalog_configuration_id;
+    if (!configId) continue; // unselectable legacy row — leave it in place
+    if (selected.has(configId)) stored.add(configId);
+    else removedIds.push(row.id);
+  }
+
+  const added: ThreadModelInsert[] = [];
+  for (const configId of selected) {
+    if (!stored.has(configId)) {
+      added.push({ thread_id: threadId, modelcatalog_configuration_id: configId });
+    }
+  }
+
+  return {
+    removedIds,
+    added,
+    unchanged: removedIds.length === 0 && added.length === 0,
+  };
+}

ui-react/src/pages/modeling/thread/MintModels.tsx

@@ -22,6 +22,8 @@ import {
   useSetThreadModelsMutation,
 } from '@/graphql/generated/modeling';
 import { useAuth } from '@/lib/auth/useAuth';
+import { diffThreadModels } from '@/lib/thread-models';
+import { useToast } from '@/components/ui/use-toast';
 import { cn } from '@/lib/utils';
 
 // ─── Types ─────────────────────────────────────────────────────────────────────
@@ -209,6 +211,7 @@ interface MintModelsProps {
 
 export function MintModels({ thread, onContinue, onThreadUpdated }: MintModelsProps) {
   const { user } = useAuth();
+  const { toast } = useToast();
   const perm = getUserPermission(thread.permissions, thread.events, user?.username ?? null);
 
   const [editMode, setEditMode] = useState(() => {
@@ -300,23 +303,30 @@ export function MintModels({ thread, onContinue, onThreadUpdated }: MintModelsPr
 
   async function handleSave() {
     if (!user?.username) return;
+
+    const changes = diffThreadModels(thread.id, thread.thread_models ?? [], selectedIds);
+    if (changes.unchanged) {
+      setEditMode(false);
+      if (onContinue) onContinue();
+      return;
+    }
+
     setSaving(true);
     try {
-      const models = Array.from(selectedIds).map((cfgId) => ({
-        thread_id: thread.id,
-        modelcatalog_configuration_id: cfgId,
-      }));
       await setThreadModels({
         variables: {
           threadId: thread.id,
-          models,
+          removedIds: changes.removedIds,
+          models: changes.added,
           userid: user.username,
           notes: notes || null,
         },
       });
       setEditMode(false);
       onThreadUpdated?.();
       if (onContinue) onContinue();
+    } catch (err) {
+      toast({ title: 'Save failed', description: String(err), variant: 'destructive' });
     } finally {
       setSaving(false);
     }

ui-react/src/pages/modeling/thread/wizard/ModelsStep.tsx

@@ -13,6 +13,8 @@ import {
   useSetThreadModelsMutation,
 } from '@/graphql/generated/modeling';
 import { useAuth } from '@/lib/auth/useAuth';
+import { diffThreadModels } from '@/lib/thread-models';
+import { useToast } from '@/components/ui/use-toast';
 import { cn } from '@/lib/utils';
 import { StepShell } from './StepShell';
 import { FilteredByBanner } from './FilteredByBanner';
@@ -123,6 +125,7 @@ export function ModelsStep({
   onEditIndicator,
 }: ModelsStepProps) {
   const { user } = useAuth();
+  const { toast } = useToast();
   const perm = getUserPermission(thread.permissions, thread.events, user?.username ?? null);
 
   const [searchText, setSearchText] = useState('');
@@ -185,17 +188,30 @@ export function ModelsStep({
 
   async function handleContinue() {
     if (!user?.username || selectedIds.size === 0) return;
+
+    const changes = diffThreadModels(thread.id, thread.thread_models ?? [], selectedIds);
+    // Nothing to write: walking back through the step must not touch the
+    // bindings the later steps have already stored against these rows.
+    if (changes.unchanged) {
+      onContinue();
+      return;
+    }
+
     setSaving(true);
     try {
-      const models = Array.from(selectedIds).map((cfgId) => ({
-        thread_id: thread.id,
-        modelcatalog_configuration_id: cfgId,
-      }));
       await setThreadModels({
-        variables: { threadId: thread.id, models, userid: user.username, notes: null },
+        variables: {
+          threadId: thread.id,
+          removedIds: changes.removedIds,
+          models: changes.added,
+          userid: user.username,
+          notes: null,
+        },
       });
       onUpdated();
       onContinue();
+    } catch (err) {
+      toast({ title: 'Save failed', description: String(err), variant: 'destructive' });
     } finally {
       setSaving(false);
     }