Stop memory writes reporting topics as lost

docs/dev/memory.md

@@ -857,6 +857,23 @@ renders the delta as a chip (`memorySizeDelta` in
 
 ## Gotchas
 
+- **A write tool must not echo `topics`.** `memory_update` and
+  `memory_reshape` select the row back with `RETURNING`, which runs
+  AFTER `clear_memory_topics_on_change` has emptied the column to
+  re-queue the row for the memory-topics curation unit. Echoing the
+  field therefore reports an empty tag list at exactly the moment the
+  model edited the text, and reads as "your tags were dropped" on a
+  write that lost nothing - the same false alarm the recipe tools hit
+  (see `./cookbook.md`). Both tools leave `topics` out of the select
+  entirely; `tests/memory_write_shape.test.ts` asserts on the column
+  list, not just the response, so re-adding it to the select fails the
+  gate. Selecting it only on `memory_update`'s confidence-only path
+  (where the trigger does not fire and the tags do survive) would be
+  worse than omitting it: the model would have no way to tell an
+  accurate empty list from a re-queued one. `memory_search` and
+  `memory_get` are the read-back paths. `memory_create` keeps its
+  `topics` because an insert never fires the trigger - a new memory
+  genuinely has no tags yet.
 - **DELETE events need the (id, user_id) replica identity.** The
   `subscribeToMemoryChanges` relay filters on `user_id`, but a
   DELETE's WAL record carries only the table's replica identity -

src/lib/tools/record_delete.schema.ts

@@ -8,7 +8,9 @@ export const recordDeleteSchema = {
     'Delete a record the user no longer wants kept. Provide id (from ' +
     'record_list or record_search). Hard delete - records are historical ' +
     'documentation, so only remove one when the user explicitly asks or ' +
-    'it is clearly a duplicate / mistake. Returns {deleted: true}.',
+    'it is clearly a duplicate / mistake. Returns {deleted: true}, or ' +
+    '{deleted: false} when no record with that id exists - check the ' +
+    'flag rather than assuming the delete landed.',
   shortDescription: 'delete a record',
   parameters: {
     type: 'object',

supabase/functions/tests/memory_write_shape.test.ts

@@ -0,0 +1,92 @@
+// Return-shape guards for the memory write tools.
+//
+// Same regression as tests/recipe_update.test.ts, found by auditing the
+// other tools for it: a write echoed the row's `topics` column, but the
+// label/data edit that triggered the write also fires
+// clear_memory_topics_on_change, which empties that column so the
+// curation unit re-tags the row. RETURNING reads the row back after the
+// trigger, so the field reported an empty list exactly when the model
+// had just edited the text - and reads as "the edit dropped your tags"
+// on a write that lost nothing.
+//
+// These assert on the SELECT column list, which is where the field
+// enters the response. A stub that returned a fixed row would pass even
+// if someone re-added `topics` to the select.
+
+import { assert, assertEquals } from '@std/assert';
+import type { SupabaseClient } from '@supabase/supabase-js';
+import type { ToolContext } from '../venice/performToolCall.ts';
+import { memoryUpdate } from '../venice/tools/memory_update.ts';
+import { memoryReshape } from '../venice/tools/memory_reshape.ts';
+
+function fakeCtx(): { ctx: ToolContext; selects: string[] } {
+  const selects: string[] = [];
+  const row = {
+    id: 'm-1',
+    label: 'likes rye',
+    data: 'Prefers rye bread.',
+    confidence: 7,
+    created_at: 't0',
+    updated_at: 't1',
+  };
+
+  const adminClient = {
+    from: () => {
+      const c: Record<string, unknown> = {};
+      for (const m of ['update', 'eq', 'insert', 'order', 'limit', 'in']) c[m] = () => c;
+      c.select = (cols?: string) => {
+        if (typeof cols === 'string') selects.push(cols);
+        return c;
+      };
+      c.single = () => Promise.resolve({ data: row, error: null });
+      c.maybeSingle = () => Promise.resolve({ data: row, error: null });
+      c.then = (res: (v: unknown) => unknown, rej?: (e: unknown) => unknown) =>
+        Promise.resolve({ data: [row], error: null }).then(res, rej);
+      return c;
+    },
+  } as unknown as SupabaseClient;
+
+  return {
+    ctx: {
+      adminClient,
+      userId: 'u-1',
+      threadId: 't-1',
+      signal: new AbortController().signal,
+      depth: 0,
+    } as ToolContext,
+    selects,
+  };
+}
+
+/** The column list the tool reads its response row back with. */
+function writeSelect(selects: string[]): string {
+  const cols = selects.find((s) => s.includes('confidence'));
+  assert(cols, `no response-row select found in ${JSON.stringify(selects)}`);
+  return cols;
+}
+
+Deno.test('memory_update does not echo the re-queued topics column', async () => {
+  const { ctx, selects } = fakeCtx();
+  const out = (await memoryUpdate.execute(
+    { id: 'm-1', data: 'Prefers rye bread.', message: 'tightened' },
+    ctx,
+  )) as Record<string, unknown>;
+  assertEquals(writeSelect(selects).includes('topics'), false);
+  assertEquals('topics' in out, false);
+  // The fields the model actually needs still come back.
+  assertEquals(out.id, 'm-1');
+  assertEquals(out.confidence, 7);
+});
+
+Deno.test('memory_reshape does not echo the re-queued topics column', async () => {
+  // A reshape always rewrites label or data, so it always fires the
+  // re-tag trigger - this field could never be anything but empty.
+  const { ctx, selects } = fakeCtx();
+  const out = (await memoryReshape.execute(
+    { id: 'm-1', data: 'Prefers rye.', message: 'de-narrated' },
+    ctx,
+  )) as Record<string, unknown>;
+  assertEquals(writeSelect(selects).includes('topics'), false);
+  assertEquals('topics' in out, false);
+  assertEquals(out.label, 'likes rye');
+});

supabase/functions/venice/tools/memory_reshape.ts

@@ -83,7 +83,13 @@ export const memoryReshape: ToolDef = {
       .update(patch)
       .eq('id', id)
       .eq('user_id', ctx.userId)
-      .select('id, label, data, confidence, topics, created_at, updated_at')
+      // No `topics`: a reshape always changes label or data, so it always
+      // fires clear_memory_topics_on_change, and the RETURNING clause
+      // reads the row back after that trigger has emptied the column.
+      // The field could only ever echo an empty list here, reading as
+      // "the reshape dropped the tags" when the curation unit has merely
+      // been asked to re-tag. Same reasoning as memory_update.
+      .select('id, label, data, confidence, created_at, updated_at')
       .single();
     if (error) throw new Error(`reshapeMemory failed: ${error.message}`);
 

supabase/functions/venice/tools/memory_update.ts

@@ -5,6 +5,17 @@
 // trigger that nulls the embedding, queuing the row for re-embedding
 // by the worker; a confidence-only patch does not. Wire schema in
 // src/lib/tools/memory_update.schema.ts.
+//
+// The echoed row deliberately omits `topics`. The same label/data change
+// also fires clear_memory_topics_on_change, which empties the column so
+// the memory-topics curation unit re-tags the row, and the RETURNING
+// clause reads the row back AFTER that trigger - so the field would read
+// as an empty list precisely when the model had just edited the text.
+// Reporting it invites "your tags are gone" on a write that lost
+// nothing. Selecting it only on the confidence-only path (where it does
+// survive) would be worse: the model has no way to tell an accurate
+// empty list from a re-queued one. memory_search and memory_get are the
+// read-back paths once the unit has caught up.
 
 import { registerTool, type ToolContext, type ToolDef } from '../performToolCall.ts';
 import { appendMemoryChangelog } from './_memory_changelog.ts';
@@ -90,7 +101,7 @@ export const memoryUpdate: ToolDef = {
       .update(patch)
       .eq('id', id)
       .eq('user_id', ctx.userId)
-      .select('id, label, data, confidence, topics, created_at, updated_at')
+      .select('id, label, data, confidence, created_at, updated_at')
       .single();
     if (error) throw new Error(`updateMemory failed: ${error.message}`);