diff --git a/docs/dev/cookbook.md b/docs/dev/cookbook.md index 30d61fa0..a24806cf 100644 --- a/docs/dev/cookbook.md +++ b/docs/dev/cookbook.md @@ -50,11 +50,11 @@ unaffected. schemas (name, description, JSON Schema parameters). Schema only; the browser never executes a recipe tool. - `supabase/functions/venice/tools/recipe_save.ts`, `recipe_list.ts`, - `recipe_get.ts`, `recipe_update.ts`, `recipe_delete.ts`, and + `recipe_get.ts`, `recipe_delete.ts`, and `recipe_photos.ts` (which carries all four photo verbs) — the implementations, dispatched function-side against the admin client. `_recipe_helpers.ts` holds `readRecipePhotoMeta`, the "newest - version's link set" read that `recipe_get` and `recipe_update` + version's link set" read that `recipe_get` and the save tool's edit form both answer photo questions with. Mutating tools reach the UI through the realtime relay, not `notifyCookbookChanged` - see the relay gotcha below. @@ -169,7 +169,7 @@ unaffected. fetches from Supabase; subsequent switches are free). Clicking a row opens the Cookbook modal on the detail pane for that id via the `initialRecipeId` prop. -- **LLM tool calls** - `recipe_save / list / get / update / delete`, +- **LLM tool calls** - `recipe_save (create or edit) / list / get / delete`, grouped into the `cooking` toolbox. Every tool is declared on every request (no gating); the model calls them directly. @@ -187,7 +187,14 @@ unaffected. partial `recipes_user_upcoming_idx (user_id) where upcoming` and `recipes_user_favorite_idx (user_id) where favorite` to keep the "list upcoming" / "list favorites" paths cheap while most rows - aren't bookmarked. + aren't bookmarked. `recipes_user_title_unique` - a unique index + on `(user_id, lower(btrim(title)))`: duplicate titles read as + bugs (two cards that drift apart as one gets edited), and the + uniqueness key is what the `recipe_save` tool's natural-key + dedup probes against - an exact-title create routes to that + recipe's update, a near-match is refused with candidates named. + Enforced as an index because Postgres only accepts expressions in + index form. - RLS: four self-* policies (select / insert / update / delete), same shape as `memories`. - **Bookmark flags** (`upcoming`, `favorite`): not in @@ -308,7 +315,7 @@ unaffected. - A mutating tool's response must describe state it actually read back, not the shape the caller asked for. See the echoed-row gotcha below for the two fields this went wrong on. -- **The rating is user-only.** `recipe_update` accepts no `rating` +- **The rating is user-only.** `recipe_save` accepts no `rating` argument and always passes `p_set_rating: false`; a call that carries one is rejected with an explanatory error. The rating is a user evaluation of a cooked dish, so it moves only through the UI @@ -326,7 +333,7 @@ Two writes per mutation, one transaction: both plpgsql RPCs, so either both rows land or neither does. `change_message` is required at the `SupabaseService.createRecipe` / -`updateRecipe` layer (and on `recipe_update`, where a meaningful delta +`updateRecipe` layer (and on the save tool's edit form, where a meaningful delta description exists). The `recipe_save` tool is the one exception: a save is always a recipe's first version, so an omitted message has no delta to describe and the tool defaults it to `"Initial version"` @@ -353,7 +360,7 @@ under a megabyte). **Atomicity**: `recipe_update_with_version` takes a `for update` lock on the parent row before snapshotting, so concurrent writers (the user editing in the modal while the model also calls -`recipe_update`) serialize. The first writer commits its snapshot; +the save tool's edit form) serialize. The first writer commits its snapshot; the second sees the post-first-commit state and snapshots that. No gaps in the history chain, no surprise overwrites. @@ -366,10 +373,10 @@ recoverable. **No history LLM tools** (deliberately): the model has no `recipe_versions_list` or `recipe_revert` tool, only the existing -`recipe_save` / `recipe_update` / etc. History viewing and revert +`recipe_save` / etc. History viewing and revert are user-directed UX flows; letting the model revert without an explicit user prompt is a footgun without a clear win. The model -can already author whatever content it wants via `recipe_update`, +can already author whatever content it wants via the save tool's edit form, and the user has revert in the modal. Revisit if a need surfaces. **Backfill**: existing recipes that predate the rollout get one @@ -578,7 +585,7 @@ keystrokes; the LLM tool path keeps using `listRecipes`. their own form-row between the change-message field and the cooklang+preview panes. - **A tool's echoed row is a claim about live state - read it back.** - `recipe_update` answered with a hardcoded `photos: []` and echoed the + the edit form answered with a hardcoded `photos: []` and echoed the `topics` column, and both read as data loss to the model, which relayed "your photos and tags are gone" to the user after an edit that had preserved every one of them. Photos: the RPC inherits the @@ -593,7 +600,7 @@ keystrokes; the LLM tool path keeps using `listRecipes`. only ever means "re-queued." `recipe_save`'s `photos: []` is a different case and stays: a create passes `p_image_ids: null`, so the recipe genuinely has no photos yet. - `tests/recipe_update.test.ts` guards both. + `tests/recipe_save_edit_form.test.ts` guards both. - **Photo IDs are stable across versions.** A photo upserted into `recipe_images` keeps the same id forever for that user; reordering or appending changes the link rows, not the image @@ -652,7 +659,7 @@ keystrokes; the LLM tool path keeps using `listRecipes`. markdown exports append " (optional)" to the bullet, and step prose shows just the name. Ingredients only - cooklang-rs also allows `#?cookware`, but nak's flat cookware aside has nothing to - hang optionality off. The `recipe_save` / `recipe_update` tool + hang optionality off. The `recipe_save` tool descriptions teach the model the syntax; keep them in sync if the rendering changes. - **Dash-only section reset.** A line whose non-whitespace content is diff --git a/docs/user/cookbook.md b/docs/user/cookbook.md index aef6310d..a7e13f25 100644 --- a/docs/user/cookbook.md +++ b/docs/user/cookbook.md @@ -153,7 +153,9 @@ Cooklang form. If Nak tells you it can't save a recipe, check that the recipe text actually made it into the chat - the tool needs the ingredients and -steps to come from somewhere. +steps to come from somewhere. Recipe titles are unique - asking Nak +to save "the same recipe" twice updates the existing card rather +than creating a second one. ## Jumping around a recipe diff --git a/src/lib/cooklang.ts b/src/lib/cooklang.ts index 95c73ee4..f3b36ef3 100644 --- a/src/lib/cooklang.ts +++ b/src/lib/cooklang.ts @@ -711,7 +711,7 @@ export type { RecipeHtmlOptions, RecipeTocSection, RecipeTocEntry } from './cook /** * Catch the LLM-authoring quirks the parser tolerates but the renderer - * can't make readable. Called from `recipe_save` and `recipe_update` + * can't make readable. Called from the recipe_save tool (both forms) * BEFORE the write hits the DB, so a malformed save fails at the tool * surface and the LLM gets a corrective error it can act on — far * cheaper than silently storing source that renders wrong and waiting diff --git a/src/lib/recipe-limits.ts b/src/lib/recipe-limits.ts index e75b3a5b..cc8c435e 100644 --- a/src/lib/recipe-limits.ts +++ b/src/lib/recipe-limits.ts @@ -1,6 +1,6 @@ /** * Recipe length limits. Lifted out of `cooklang.ts` so the always-on - * `recipe_save` / `recipe_update` tool schemas can reach them without + * `recipe_save` tool schema can reach them without * pulling the 14 kB Cooklang parser into the main chunk - the parser * is needed only on the Cookbook screen (lazy) and at recipe-save * time (also reached via lazy tool impls). diff --git a/src/lib/supabase/cookbook.ts b/src/lib/supabase/cookbook.ts index c5bdb10b..1e07041e 100644 --- a/src/lib/supabase/cookbook.ts +++ b/src/lib/supabase/cookbook.ts @@ -346,7 +346,7 @@ export async function createRecipe( /** * Partial update. Caller guarantees at least one field in `patch` - * is set - enforced by the recipe_update tool and the Cookbook + * is set - enforced by the recipe_save tool and the Cookbook * Edit pane before this method runs. Goes through * `recipe_update_with_version` so the prior state is snapshotted * into `recipe_versions` in the same transaction. `changeMessage` diff --git a/src/lib/supabase/types/cookbook.ts b/src/lib/supabase/types/cookbook.ts index f564551a..cfaf52e0 100644 --- a/src/lib/supabase/types/cookbook.ts +++ b/src/lib/supabase/types/cookbook.ts @@ -73,7 +73,7 @@ export interface Recipe { * the trail of past states the user can browse and revert to. * * `change_message` is required - the UI Edit form and the LLM - * `recipe_save` / `recipe_update` tools all force a non-empty value + * `recipe_save` tool (both forms) all force a non-empty value * before the RPC is called. */ export interface RecipeVersion { diff --git a/src/lib/tools/index.ts b/src/lib/tools/index.ts index d4e4ce0f..2d627a43 100644 --- a/src/lib/tools/index.ts +++ b/src/lib/tools/index.ts @@ -78,7 +78,6 @@ import { conversationGetSchema } from './conversation_get.schema'; import { recipeListSchema } from './recipe_list.schema'; import { recipeGetSchema } from './recipe_get.schema'; import { recipeSaveSchema } from './recipe_save.schema'; -import { recipeUpdateSchema } from './recipe_update.schema'; import { recipeDeleteSchema } from './recipe_delete.schema'; import { recipePhotosAttachSchema } from './recipe_photos_attach.schema'; import { recipePhotosRemoveSchema } from './recipe_photos_remove.schema'; @@ -150,7 +149,6 @@ const conversationGet = serverSideTool(conversationGetSchema); const recipeList = serverSideTool(recipeListSchema); const recipeGet = serverSideTool(recipeGetSchema); const recipeSave = serverSideTool(recipeSaveSchema); -const recipeUpdate = serverSideTool(recipeUpdateSchema); const recipeDelete = serverSideTool(recipeDeleteSchema); const recipePhotosAttach = serverSideTool(recipePhotosAttachSchema); const recipePhotosRemove = serverSideTool(recipePhotosRemoveSchema); @@ -310,7 +308,6 @@ export const cookingToolbox: Toolbox = { 'recipe_get) are always-on; this toolbox carries the writes.', tools: [ recipeSave, - recipeUpdate, recipeDelete, recipePhotosAttach, recipePhotosRemove, diff --git a/src/lib/tools/recipe_photo_label_set.schema.ts b/src/lib/tools/recipe_photo_label_set.schema.ts index f29908ae..d84d7735 100644 --- a/src/lib/tools/recipe_photo_label_set.schema.ts +++ b/src/lib/tools/recipe_photo_label_set.schema.ts @@ -10,12 +10,9 @@ export const recipePhotoLabelSetSchema = { "Set or clear captions on a recipe's existing photos. labels is " + 'an array of {photo_id, label} pairs; each sets the caption to ' + 'the given string, or clears it when label is null/empty. Every ' + - 'photo_id must be on the recipe (use recipe_get to find ids). ' + - 'Photos not named keep their existing captions. Max 200 chars per ' + - 'caption. To add or remove photos use recipe_photos_attach or ' + - 'recipe_photos_remove. change_message REQUIRED. Returns ' + - "{recipe_id, photos: [{id, position, label}, ...]} - the recipe's " + - 'full ordered photo set with the new captions.', + 'photo_id must be on the recipe (from recipe_get); photos not ' + + 'named keep their captions. Returns the full ordered photo set ' + + 'with the new captions.', shortDescription: 'set or clear photo captions on a recipe', parameters: { type: 'object', @@ -33,15 +30,12 @@ export const recipePhotoLabelSetSchema = { photo_id: { type: 'string', minLength: 1, - description: - 'Photo id to retitle. Must be on the recipe (use ' + - 'recipe_get to find ids).', + description: 'Photo id to retitle (from recipe_get).', }, label: { type: ['string', 'null'], maxLength: RECIPE_PHOTO_LABEL_MAX_CHARS, - description: - 'New caption, or null/empty to clear. Max 200 chars.', + description: 'New caption, or null/empty to clear.', }, }, required: ['photo_id'], @@ -56,8 +50,7 @@ export const recipePhotoLabelSetSchema = { minLength: 1, maxLength: 500, description: - 'One-line history note; lands in the recipe changelog the user reviews. Examples: "Captioned the finished ' + - 'plate", "Cleared the obsolete progress-shot caption".', + 'One-line history note; lands in the recipe changelog the user reviews.', }, }, required: ['recipe_id', 'labels', 'change_message'], diff --git a/src/lib/tools/recipe_photos_attach.schema.ts b/src/lib/tools/recipe_photos_attach.schema.ts index 6964947e..6fee5ea1 100644 --- a/src/lib/tools/recipe_photos_attach.schema.ts +++ b/src/lib/tools/recipe_photos_attach.schema.ts @@ -9,16 +9,9 @@ export const recipePhotosAttachSchema = { 'filenames lists conversation-attachment filenames in display ' + 'order (must match exactly, case-sensitive); ' + 'each must be live (not expired). Photos already on the recipe ' + - 'are not duplicated; the array appends. Optional labels is ' + - 'parallel-indexed with filenames (labels[i] captions ' + - 'filenames[i]); pass empty/null for no caption, omit the array ' + - 'entirely when no photo gets a caption. Re-attaching a filename ' + + 'are not duplicated; the array appends. Re-attaching a filename ' + 'already on the recipe with a non-empty label updates that ' + - "photo's caption. Use recipe_photos_remove to drop, " + - 'recipe_photos_reorder to reorder, recipe_photo_label_set to ' + - 'recaption photos already on the recipe. change_message REQUIRED. ' + - 'Returns {recipe_id, photos: [{id, position, label}, ...]} - ' + - 'the post-attach full ordered set.', + "photo's caption. Returns the post-attach full ordered photo set.", shortDescription: 'attach conversation images to a recipe', parameters: { type: 'object', @@ -39,18 +32,15 @@ export const recipePhotosAttachSchema = { type: 'array', items: { type: ['string', 'null'], maxLength: 200 }, description: - 'Optional captions parallel-indexed with filenames. Length ' + - 'MUST match filenames when provided; pass empty/null per ' + - 'photo for no caption. Omit the field entirely if none get ' + - 'captions.', + 'Optional captions, parallel-indexed with filenames (same ' + + 'length). Omit if none get captions.', }, change_message: { type: 'string', minLength: 1, maxLength: 500, description: - 'One-line history note; lands in the recipe changelog the user reviews. Examples: "Added the finished plate ' + - 'photo", "Saved the dough progress shot".', + 'One-line history note; lands in the recipe changelog the user reviews.', }, }, required: ['recipe_id', 'filenames', 'change_message'], diff --git a/src/lib/tools/recipe_photos_remove.schema.ts b/src/lib/tools/recipe_photos_remove.schema.ts index 89b0a042..6568effa 100644 --- a/src/lib/tools/recipe_photos_remove.schema.ts +++ b/src/lib/tools/recipe_photos_remove.schema.ts @@ -5,12 +5,10 @@ export const recipePhotosRemoveSchema = { name: 'recipe_photos_remove', description: - 'Remove one or more photos from a recipe by photo id (the id ' + - "field on each entry of recipe_get's photos array). Every id " + - 'must be on the recipe; an unknown id fails the call rather than ' + - 'silently skipping. change_message REQUIRED. Returns ' + - '{recipe_id, photos: [{id, position, label}, ...]} - the ' + - 'post-removal full ordered set with surviving captions preserved.', + 'Remove one or more photos from a recipe by photo id (from ' + + "recipe_get's photos array). Every id must be on the recipe; an " + + 'unknown id fails the call rather than silently skipping. Returns ' + + 'the post-removal full ordered photo set.', shortDescription: 'remove photos from a recipe by id', parameters: { type: 'object', @@ -31,8 +29,7 @@ export const recipePhotosRemoveSchema = { minLength: 1, maxLength: 500, description: - 'One-line history note; lands in the recipe changelog the user reviews. Examples: "Removed the blurry first ' + - 'attempt", "Dropped the redundant overhead shot".', + 'One-line history note; lands in the recipe changelog the user reviews.', }, }, required: ['recipe_id', 'photo_ids', 'change_message'], diff --git a/src/lib/tools/recipe_photos_reorder.schema.ts b/src/lib/tools/recipe_photos_reorder.schema.ts index 9b318745..18ecb693 100644 --- a/src/lib/tools/recipe_photos_reorder.schema.ts +++ b/src/lib/tools/recipe_photos_reorder.schema.ts @@ -5,14 +5,11 @@ export const recipePhotosReorderSchema = { name: 'recipe_photos_reorder', description: - "Set a recipe's photo display order. photo_ids MUST be a " + + "Set a recipe's photo display order. photo_ids must be a " + "permutation of the recipe's current photo set (every id present, " + - 'no missing, no extras, no duplicates). Call recipe_get first to ' + - 'read the current order. To add or remove use ' + - 'recipe_photos_attach or recipe_photos_remove; to recaption use ' + - 'recipe_photo_label_set. Captions travel with their photos. ' + - 'change_message REQUIRED. Returns {recipe_id, photos: [{id, ' + - 'position, label}, ...]} with positions renumbered from 0.', + 'no extras, no duplicates; call recipe_get to read the current ' + + 'order). Captions travel with their photos. Returns the reordered ' + + 'set with positions renumbered from 0.', shortDescription: 'reorder a recipe\'s photos', parameters: { type: 'object', @@ -34,8 +31,7 @@ export const recipePhotosReorderSchema = { minLength: 1, maxLength: 500, description: - 'One-line history note; lands in the recipe changelog the user reviews. Examples: "Moved the finished plate ' + - 'first", "Grouped prep shots before the served photo".', + 'One-line history note; lands in the recipe changelog the user reviews.', }, }, required: ['recipe_id', 'photo_ids', 'change_message'], diff --git a/src/lib/tools/recipe_save.schema.ts b/src/lib/tools/recipe_save.schema.ts index 64fab6c6..8f99c3c1 100644 --- a/src/lib/tools/recipe_save.schema.ts +++ b/src/lib/tools/recipe_save.schema.ts @@ -1,23 +1,36 @@ /** - * Schema-only export for recipe_save. Impl lives in `./recipe_save`. + * Schema-only export for recipe_save - the create/update-merged recipe + * write (the routing contract is the same id-optional upsert shape as + * wiki_save; there is no shared runtime module because each resource's + * conditional validation differs - see supabase/functions/venice/ + * tools/recipe_save.ts). Impl lives in the edge function, which also + * self-registers the tool for dispatch. * * Carries a `formatArgs` override read by the tool-call detail - * panel (`src/components/ToolCalls.svelte` via - * `src/lib/ui/tool-calls.ts`). The generic JSON-as-markdown - * formatter would render the cooklang source as a fenced block - * automatically because it contains newlines, but the surrounding - * shape (a top-level bullet for every other field with cooklang - * buried among them) reads worse than promoting the cooklang - * block to a labelled section below the metadata. The override - * orders the fields the way a reader would scan them - activity, - * title, source, change_message, then the recipe body + * panel (src/components/ToolCalls.svelte via src/lib/ui/tool-calls.ts). + * The generic JSON-as-markdown formatter would render the cooklang + * source as a fenced block automatically because it contains newlines, + * but the surrounding shape (a top-level bullet for every other field + * with cooklang buried among them) reads worse than promoting the + * cooklang block to a labelled section below the metadata. The + * override orders the fields the way a reader would scan them - + * activity, id, title, source, change_message, then the recipe body * itself. + * + * The full Cooklang authoring spec lives verbatim in this + * description (cooklang is poorly represented in model training + * data). Both forms of the tool need it, and under + * every-tool-declared-every-request there is exactly one schema on + * the wire for it. */ import { MAX_RECIPE_COOKLANG_CHARS, MAX_RECIPE_TITLE_CHARS } from '../recipe-limits'; function formatRecipeSaveArgs(args: Record): string { const lines: string[] = []; const scalar: Array<[string, string]> = []; + if (typeof args.id === 'string' && args.id) { + scalar.push(['id', String(args.id)]); + } for (const key of ['title', 'source', 'source_url', 'change_message'] as const) { const v = args[key]; if (v === undefined || v === null || v === '') continue; @@ -48,23 +61,26 @@ function formatRecipeSaveArgs(args: Record): string { export const recipeSaveSchema = { name: 'recipe_save', description: - "Save a new recipe. cooklang is the raw Cooklang source " + - '(https://cooklang.org/docs/spec/): @ingredient{qty%unit}, ' + - `#cookware{}, ~timer{d%unit}, >> metadata: value (max ${MAX_RECIPE_COOKLANG_CHARS} ` + - 'chars). Group long recipes with `== Section ==` or `# Section` ' + - 'headers. Two authoring styles supported and mixable: (a) pure ' + - 'Cooklang (each line is an instruction with inline ingredients); ' + - "(b) cookbook-style (a line whose first non-whitespace char is `@` " + - 'is an ingredient DECLARATION, not numbered as an instruction; a ' + - 'dash-only line like `--` ends the declaration block so prose ' + - 'instructions below render as a flat numbered list). Wrap a long ' + - 'instruction across lines by prefixing continuations with `> `. ' + - 'Inline emphasis is supported in step text: `**bold**`, ' + - '`*italic*`, and `_italic_` render as styled spans. Backtick code ' + - 'spans are NOT rendered - they show as literal backticks, so ' + - "don't use them. For durations, prefer the Cooklang timer syntax " + - '`~{N%unit}` (e.g. `~{4-5%hours}`) so the duration also ' + - 'contributes to the timers list; wrapping it in `**...**` for ' + + "Save a recipe: create a new one, or update an existing one by id. " + + 'Omit id to create (title + cooklang required); pass id (from ' + + 'recipe_list) to update, providing only the fields that change - ' + + 'pass null for source / source_url to clear them. cooklang is the ' + + 'raw Cooklang source (https://cooklang.org/docs/spec/): ' + + '@ingredient{qty%unit}, #cookware{}, ~timer{d%unit}, ' + + `>> metadata: value (max ${MAX_RECIPE_COOKLANG_CHARS} chars). Group ` + + 'long recipes with `== Section ==` or `# Section` headers. Two ' + + 'authoring styles supported and mixable: (a) pure Cooklang (each ' + + 'line is an instruction with inline ingredients); (b) cookbook-style ' + + "(a line whose first non-whitespace char is `@` is an ingredient " + + 'DECLARATION, not numbered as an instruction; a dash-only line like ' + + '`--` ends the declaration block so prose instructions below render ' + + 'as a flat numbered list). Wrap a long instruction across lines by ' + + 'prefixing continuations with `> `. Inline emphasis is supported in ' + + 'step text: `**bold**`, `*italic*`, and `_italic_` render as styled ' + + 'spans. Backtick code spans are NOT rendered - they show as literal ' + + "backticks, so don't use them. For durations, prefer the Cooklang " + + 'timer syntax `~{N%unit}` (e.g. `~{4-5%hours}`) so the duration ' + + 'also contributes to the timers list; wrapping it in `**...**` for ' + 'emphasis is fine but the `~` is what makes it a timer. For an ' + 'ingredient with a modifier, write the whole phrase as a single ' + 'multi-word braced name: `@pre-minced garlic{1%tbsp}`, NEVER ' + @@ -72,15 +88,24 @@ export const recipeSaveSchema = { 'ingredient entries). Mark an OPTIONAL ingredient with `?` right ' + 'after the `@` (`@?cilantro{2%tbsp}`, bare `@?cilantro`) - it ' + 'renders with an "(optional)" tag in the ingredient list. For ' + - 'alternatives ("use X or Y"), only the ' + - 'primary ingredient gets `@`; write the substitute as plain prose. ' + - 'change_message lands in the recipe history; it is optional here ' + - 'and defaults to "Initial version" since a save is always the ' + - 'first version. Returns {id, title, updated_at}.', + "alternatives (\"use X or Y\"), only the primary ingredient gets " + + '`@`; write the substitute as plain prose. The star rating is the ' + + "user's own verdict and is not editable here - only they can set " + + 'or clear it, from the recipe card. change_message: optional on ' + + 'create (defaults to "Initial version"), required on update - it ' + + 'lands in the recipe history the user reviews. Returns the saved ' + + 'row plus the current photo list, which this tool never changes - ' + + 'use the recipe_photos_* tools to edit photos.', shortDescription: 'save a recipe to the cookbook', parameters: { type: 'object', properties: { + id: { + type: 'string', + description: + 'UUID of the recipe to update (from recipe_list). Omit to ' + + 'create a new recipe.', + }, title: { type: 'string', minLength: 1, @@ -94,26 +119,25 @@ export const recipeSaveSchema = { description: 'Full Cooklang source.', }, source: { - type: 'string', + type: ['string', 'null'], maxLength: 400, description: - 'Optional free-form provenance (e.g. "NYT Cooking - Alison Roman").', + 'Optional free-form provenance (e.g. "NYT Cooking - Alison ' + + 'Roman"), or null to clear.', }, source_url: { - type: 'string', + type: ['string', 'null'], maxLength: 2000, - description: 'Optional URL the recipe was imported from.', + description: 'Optional URL provenance, or null to clear.', }, change_message: { type: 'string', minLength: 1, maxLength: 500, description: - 'Optional one-line history note; lands in the recipe changelog ' + - 'the user reviews. Defaults to "Initial ' + - 'version" when omitted - only set it when you have something ' + - 'more specific than "first save" to record (e.g. "Imported ' + - 'from NYT Cooking", "Captured from prose the user pasted").', + 'One-line history note; lands in the recipe changelog the ' + + 'user reviews. Required when updating; optional on create ' + + '(defaults to "Initial version").', }, }, required: ['title', 'cooklang'], diff --git a/src/lib/tools/recipe_update.schema.ts b/src/lib/tools/recipe_update.schema.ts deleted file mode 100644 index 54005d28..00000000 --- a/src/lib/tools/recipe_update.schema.ts +++ /dev/null @@ -1,62 +0,0 @@ -/** - * Schema-only export for recipe_update. Impl lives in `./recipe_update`. - */ -import { MAX_RECIPE_COOKLANG_CHARS, MAX_RECIPE_TITLE_CHARS } from '../recipe-limits'; - -export const recipeUpdateSchema = { - name: 'recipe_update', - // The cooklang authoring rules live once, in recipe_save's - // description (cooklang is poorly represented in model training - // data, so the full spec stays verbatim there). Under - // every-tool-declared-every-request both schemas ride the same - // request, so a cross-reference is safe and saves ~600 chars per - // request. - description: - 'Update a recipe by id. Provide at least one of title, cooklang, ' + - 'source, or source_url; omit a field to leave it unchanged. ' + - 'Pass ' + - 'null for source / source_url to clear them. The star rating is ' + - "the user's own verdict and is not editable here - only they can " + - 'set or clear it, from the recipe card. cooklang ' + - `capped at ${MAX_RECIPE_COOKLANG_CHARS} chars; use the same Cooklang ` + - 'authoring rules recipe_save describes. change_message is REQUIRED and lands in the recipe ' + - "history. Returns the updated row plus the recipe's current photo " + - 'list, which this tool never changes - use the recipe_photos_* ' + - 'tools to edit photos.', - shortDescription: 'edit a saved recipe', - parameters: { - type: 'object', - properties: { - id: { - type: 'string', - description: 'UUID of the recipe (from recipe_list).', - }, - title: { type: 'string', minLength: 1, maxLength: MAX_RECIPE_TITLE_CHARS }, - cooklang: { - type: 'string', - minLength: 1, - maxLength: MAX_RECIPE_COOKLANG_CHARS, - }, - source: { - type: ['string', 'null'], - maxLength: 400, - description: 'Free-form provenance, or null to clear.', - }, - source_url: { - type: ['string', 'null'], - maxLength: 2000, - description: 'URL provenance, or null to clear.', - }, - change_message: { - type: 'string', - minLength: 1, - maxLength: 500, - description: - 'One-line history note; lands in the recipe changelog the user reviews. Examples: "Fixed servings ' + - 'metadata", "Removed tahini per user dietary note".', - }, - }, - required: ['id', 'change_message'], - additionalProperties: false, - }, -} as const; diff --git a/src/lib/tools/record_file_attach.schema.ts b/src/lib/tools/record_file_attach.schema.ts index d8cb9671..fd6b5615 100644 --- a/src/lib/tools/record_file_attach.schema.ts +++ b/src/lib/tools/record_file_attach.schema.ts @@ -11,12 +11,10 @@ export const recordFileAttachSchema = { name: 'record_file_attach', description: 'Attach a file from THIS conversation to a wiki record, by its ' + - 'filename. Works for any file the conversation holds - a file the ' + - 'user uploaded or an image you generated. The bytes are copied into ' + - 'permanent record storage, so the file stays on the record even after ' + - 'the chat attachment expires. Use this to put crumb photos, scanned ' + - 'cards, or generated images onto the record that documents them. The ' + - 'file must still be live in the thread (an expired attachment errors).', + 'filename - a user upload or a generated image. The bytes are copied ' + + 'into permanent record storage, so the file stays on the record even ' + + 'after the chat attachment expires. The file must still be live in ' + + 'the thread (an expired attachment errors).', shortDescription: 'attach a conversation file to a record', parameters: { type: 'object', diff --git a/src/lib/tools/record_link_create.schema.ts b/src/lib/tools/record_link_create.schema.ts index d85f2407..b50e0ea5 100644 --- a/src/lib/tools/record_link_create.schema.ts +++ b/src/lib/tools/record_link_create.schema.ts @@ -8,12 +8,10 @@ import { MAX_RECORD_LINK_LABEL_CHARS } from '../wiki'; export const recordLinkCreateSchema = { name: 'record_link_create', description: - 'Link one record to another with a short relationship label, e.g. ' + - '"based on", "supersedes", "same dough". The link is DIRECTED ' + - '(from -> to): create it from the newer/derived record to the one it ' + - 'builds on. Re-linking the same pair updates the label. Use this to ' + - 'record that one attempt is a follow-up to another. Both records must ' + - 'belong to the user; link only when the relationship is explicit.', + 'Link one record to another with a short relationship label. The ' + + 'link is DIRECTED (from -> to): create it from the newer/derived ' + + 'record to the one it builds on. Re-linking the same pair updates ' + + 'the label. Link only when the relationship is explicit.', shortDescription: 'link two records with a label', parameters: { type: 'object', diff --git a/supabase/functions/tests/memory_write_shape.test.ts b/supabase/functions/tests/memory_write_shape.test.ts index f0d50d0c..91efd5d8 100644 --- a/supabase/functions/tests/memory_write_shape.test.ts +++ b/supabase/functions/tests/memory_write_shape.test.ts @@ -1,6 +1,6 @@ // Return-shape guards for the memory write tools. // -// Same regression as tests/recipe_update.test.ts, found by auditing the +// Same regression as tests/recipe_save_edit_form.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 diff --git a/supabase/functions/tests/recipe_save.test.ts b/supabase/functions/tests/recipe_save.test.ts index 1162fe57..c12f0bec 100644 --- a/supabase/functions/tests/recipe_save.test.ts +++ b/supabase/functions/tests/recipe_save.test.ts @@ -10,12 +10,34 @@ import type { SupabaseClient } from '@supabase/supabase-js'; import type { ToolContext } from '../venice/performToolCall.ts'; import { recipeSave } from '../venice/tools/recipe_save.ts'; -function fakeCtx(): { +function fakeCtx(opts: { existingTitle?: string } = {}): { ctx: ToolContext; rpcCalls: Array>; } { const rpcCalls: Array> = []; + // The dedup probe runs before the RPC on the create path; its ilike + // chain resolves through .then to an empty row set, so a fresh title + // proceeds to the create RPC. const adminClient = { + from: (_table: string) => { + const c: Record = {}; + // order/maybeSingle: the edit form's readRecipePhotoMeta follow-up + // read uses the same builder shape. + for (const m of ['eq', 'ilike', 'limit', 'order', 'maybeSingle']) c[m] = () => c; + c.select = () => c; + // The awaited chain never leaves this object (every chained + // method returns `c`), so `then` lives directly on it and + // resolves exactly once with the probe's row set. A Proxy that + // intercepts `then` does NOT work here: only the outer from() + // call is proxied, and the chained calls run on the raw target, + // bypassing the trap. + const rows = opts.existingTitle + ? [{ id: 'r-1', title: opts.existingTitle }] + : []; + (c as { then: unknown }).then = (resolve: (v: unknown) => unknown) => + Promise.resolve({ data: rows }).then(resolve); + return c; + }, rpc: (_name: string, args: Record) => { rpcCalls.push(args); return Promise.resolve({ @@ -47,7 +69,36 @@ Deno.test('recipe_save refuses to set the star rating', async () => { await assertRejects( () => recipeSave.execute({ ...ARGS, rating: 5 }, ctx), Error, - 'rating is not settable by this tool', + 'rating is not editable by this tool', + ); + assertEquals(rpcCalls.length, 0); +}); + +Deno.test('recipe_save create with an exact-title match routes to update', async () => { + // The natural-key heuristic: a create whose title exactly matches + // (ci) an existing recipe updates that recipe instead of bouncing + // off the unique index or silently duplicating. + const { ctx, rpcCalls } = fakeCtx({ existingTitle: 'Meatballs' }); + const result = await recipeSave.execute( + { title: 'meatballs', cooklang: 'Mix @pork{500%g}.', change_message: 'fix salt' }, + ctx, + ); + // The RPC bundle is the UPDATE shape (p_id present), and the result + // carries the flag so the model can tell which path fired. + assertEquals(rpcCalls.length, 1); + assertEquals(rpcCalls[0].p_id, 'r-1'); + assertEquals( + (result as { matched_existing?: boolean }).matched_existing, + true, + ); +}); + +Deno.test('recipe_save create with a near-match title is refused', async () => { + const { ctx, rpcCalls } = fakeCtx({ existingTitle: 'Weeknight Chili' }); + await assertRejects( + () => recipeSave.execute({ title: 'Chili', cooklang: 'x' }, ctx), + Error, + 'a similar row already exists', ); assertEquals(rpcCalls.length, 0); }); diff --git a/supabase/functions/tests/recipe_update.test.ts b/supabase/functions/tests/recipe_save_edit_form.test.ts similarity index 88% rename from supabase/functions/tests/recipe_update.test.ts rename to supabase/functions/tests/recipe_save_edit_form.test.ts index ed06b733..8eb70b77 100644 --- a/supabase/functions/tests/recipe_update.test.ts +++ b/supabase/functions/tests/recipe_save_edit_form.test.ts @@ -1,4 +1,4 @@ -// Return-shape guards for venice/tools/recipe_update.ts. +// Return-shape guards for the edit form of venice/tools/recipe_save.ts. // // The regression these exist for: a scalar edit (title / cooklang / // source / rating) inherits the recipe's photo links onto the new @@ -10,7 +10,7 @@ import { assertEquals, assertRejects } from '@std/assert'; import type { SupabaseClient } from '@supabase/supabase-js'; import type { ToolContext } from '../venice/performToolCall.ts'; -import { recipeUpdate } from '../venice/tools/recipe_update.ts'; +import { recipeSave } from '../venice/tools/recipe_save.ts'; interface PhotoLink { position: number; @@ -79,7 +79,7 @@ Deno.test('recipe_update reports the photos it carried forward', async () => { { position: 2, image_id: 'img-c', label: null }, ], }); - const out = (await recipeUpdate.execute(ARGS, ctx)) as { + const out = (await recipeSave.execute(ARGS, ctx)) as { photos: Array<{ id: string; position: number; label: string | null }>; }; // Sorted by position, not by the order the join happened to return. @@ -98,20 +98,20 @@ Deno.test('recipe_update omits the re-queued topics column', async () => { const { ctx } = fakeCtx({ rpcRow: { id: 'r-1', title: 'Meatballs', rating: 4, topics: [] }, }); - const out = (await recipeUpdate.execute(ARGS, ctx)) as Record; + const out = (await recipeSave.execute(ARGS, ctx)) as Record; assertEquals('topics' in out, false); assertEquals(out.rating, 4); }); Deno.test('recipe_update reports no photos when the recipe has none', async () => { const { ctx } = fakeCtx({ links: [] }); - const out = (await recipeUpdate.execute(ARGS, ctx)) as { photos: unknown[] }; + const out = (await recipeSave.execute(ARGS, ctx)) as { photos: unknown[] }; assertEquals(out.photos, []); }); Deno.test('recipe_update survives a recipe with no version row', async () => { const { ctx } = fakeCtx({ noVersionRow: true }); - const out = (await recipeUpdate.execute(ARGS, ctx)) as { photos: unknown[] }; + const out = (await recipeSave.execute(ARGS, ctx)) as { photos: unknown[] }; assertEquals(out.photos, []); }); @@ -122,7 +122,7 @@ Deno.test('recipe_update leaves the photo set alone', async () => { const { ctx, rpcCalls } = fakeCtx({ links: [{ position: 0, image_id: 'img-a', label: null }], }); - await recipeUpdate.execute(ARGS, ctx); + await recipeSave.execute(ARGS, ctx); assertEquals(rpcCalls.length, 1); assertEquals(rpcCalls[0].p_set_image_ids, false); assertEquals(rpcCalls[0].p_image_ids, null); @@ -131,7 +131,7 @@ Deno.test('recipe_update leaves the photo set alone', async () => { Deno.test('recipe_update still rejects a patch with nothing to change', async () => { const { ctx } = fakeCtx({}); await assertRejects( - () => recipeUpdate.execute({ id: 'r-1', change_message: 'noop' }, ctx), + () => recipeSave.execute({ id: 'r-1', change_message: 'noop' }, ctx), Error, 'provide at least one of', ); @@ -144,7 +144,7 @@ Deno.test('recipe_update refuses to touch the star rating', async () => { // reported to the user as a rating change that never happened. const { ctx, rpcCalls } = fakeCtx({}); await assertRejects( - () => recipeUpdate.execute({ ...ARGS, rating: 5 }, ctx), + () => recipeSave.execute({ ...ARGS, rating: 5 }, ctx), Error, 'rating is not editable by this tool', ); @@ -153,7 +153,7 @@ Deno.test('recipe_update refuses to touch the star rating', async () => { Deno.test('recipe_update never sets the rating on the RPC', async () => { const { ctx, rpcCalls } = fakeCtx({}); - await recipeUpdate.execute(ARGS, ctx); + await recipeSave.execute(ARGS, ctx); assertEquals(rpcCalls[0].p_set_rating, false); assertEquals(rpcCalls[0].p_rating, null); }); diff --git a/supabase/functions/venice/tools/_recipe_helpers.ts b/supabase/functions/venice/tools/_recipe_helpers.ts index 6d1de6fe..911c0a12 100644 --- a/supabase/functions/venice/tools/_recipe_helpers.ts +++ b/supabase/functions/venice/tools/_recipe_helpers.ts @@ -2,7 +2,7 @@ // // The newest `recipe_versions` row carries the recipe's live photo // links, so "which photos does this recipe have right now" is a join -// against that one row. Both recipe_get and recipe_update need the +// against that one row. Both recipe_get and the save tool's edit form need the // answer: get because the model asked for it, update because every // scalar edit inherits the previous version's links and has to report // what it carried forward. diff --git a/supabase/functions/venice/tools/index.ts b/supabase/functions/venice/tools/index.ts index 62835d29..fcf16503 100644 --- a/supabase/functions/venice/tools/index.ts +++ b/supabase/functions/venice/tools/index.ts @@ -53,7 +53,6 @@ import './recipe_get.ts'; import './recipe_list.ts'; import './recipe_photos.ts'; import './recipe_save.ts'; -import './recipe_update.ts'; import './research_docs.ts'; import './update_title.ts'; import './web_search.ts'; diff --git a/supabase/functions/venice/tools/recipe_get.ts b/supabase/functions/venice/tools/recipe_get.ts index fede2c1a..d64a9d32 100644 --- a/supabase/functions/venice/tools/recipe_get.ts +++ b/supabase/functions/venice/tools/recipe_get.ts @@ -30,7 +30,7 @@ export const recipeGet: ToolDef = { if (!recipe) return { found: false }; // Newest recipe_version row carries the current photo set; the - // shared helper owns that join (recipe_update reads it back the + // shared helper owns that join (recipe_save's edit form reads it back the // same way). Ownership was validated by the select above. const photos = await readRecipePhotoMeta(ctx.adminClient, id); diff --git a/supabase/functions/venice/tools/recipe_save.ts b/supabase/functions/venice/tools/recipe_save.ts index 488042cc..6114cee5 100644 --- a/supabase/functions/venice/tools/recipe_save.ts +++ b/supabase/functions/venice/tools/recipe_save.ts @@ -1,16 +1,34 @@ -// recipe_save (function-side port) +// recipe_save (create/update-merged recipe write) // -// Persist a new Cooklang recipe via recipe_create_with_version RPC -// (now p_user_id-aware - see schema delta). Returns the created row -// plus an empty photo array so the wire shape stays parallel with -// recipe_get / recipe_update. +// One tool behind an optional `id`: omit it to create a new recipe, +// pass it to patch an existing one. Consolidates the former +// recipe_save + recipe_update (the routing contract is the same +// id-optional upsert shape as wiki_save; there is no shared runtime +// module because each resource's conditional validation differs). +// Wire schema lives in src/lib/tools/recipe_save.schema.ts. Auth: +// b-strict. // -// Cooklang authoring-quirk validator is inlined here (small, two -// regex checks). Mirrors validateCooklangSource in src/lib/cooklang.ts - -// if that file's check list grows, mirror the additions here. +// The two halves hit different RPCs: recipe_create_with_version on a +// create, recipe_update_with_version on an edit. The RPCs use +// p_set_ + p_ pairs - explicit null clears, omission +// leaves alone - so the patch bundle is built from named fields only. +// +// The edit form never sets the photo list (p_set_image_ids=false), so +// the RPC carries the previous version's links onto the new version. +// The response still reads those links back and reports them: an edit +// that reported `photos: []` looked like it had wiped the recipe's +// photos, and the model relayed that to the user as data loss on an +// edit that had in fact preserved every photo. Changing the photo set +// is the recipe_photos_* tools' job. +// +// Cooklang authoring-quirk validator mirrors validateCooklangSource +// in src/lib/cooklang.ts - if that file's check list grows, mirror +// the additions here. import { registerTool, type ToolContext, type ToolDef } from '../performToolCall.ts'; +import { readRecipePhotoMeta } from './_recipe_helpers.ts'; import { ArgErrors } from './_validate.ts'; +import { resolveNaturalKeyMatch } from './_upsert_heuristics.ts'; // Mirror of src/lib/recipe-limits.ts - the caps the wire schema // advertises. Divergent copies here rejected schema-legal bodies. @@ -19,85 +37,270 @@ const MAX_RECIPE_COOKLANG_CHARS = 20_000; import { validateCooklangSource } from '../../_shared/cooklang-validate.ts'; -export const recipeSave: ToolDef = { - name: 'recipe_save', - async execute(args: Record, ctx: ToolContext) { - const title = typeof args.title === 'string' ? args.title.trim() : ''; - const cooklang = typeof args.cooklang === 'string' ? args.cooklang : ''; - const source = - typeof args.source === 'string' && args.source.trim().length > 0 - ? args.source.trim() - : null; - const sourceUrl = - typeof args.source_url === 'string' && args.source_url.trim().length > 0 - ? args.source_url.trim() - : null; - const errs = new ArgErrors(); - // The star rating is the user's evaluation of a dish they cooked, so - // no tool writes it - only the star control on the recipe card and - // the edit form do. A call carrying one fails loudly rather than - // dropping it silently: a silent drop reads to the model as a - // successful write, and it then tells the user a rating was saved. - if ('rating' in args) { - errs.add( - 'rating is not settable by this tool - the star rating is the ' + - "user's own evaluation and only they can set it", - ); +/** + * The star rating is the user's evaluation of a dish they cooked, so + * no tool writes it - only the star control on the recipe card does. + * A call carrying one fails loudly rather than dropping it silently: + * a silent drop reads to the model as a successful write, and it then + * tells the user a rating was saved. Both forms share this guard. + */ +function rejectRating(errs: ArgErrors, args: Record) { + if ('rating' in args) { + errs.add( + 'rating is not editable by this tool - the star rating is the ' + + "user's own evaluation and only they can set or clear it", + ); + } +} + +async function doCreate( + args: Record, + ctx: ToolContext, +): Promise { + const title = typeof args.title === 'string' ? args.title.trim() : ''; + const cooklang = typeof args.cooklang === 'string' ? args.cooklang : ''; + const source = + typeof args.source === 'string' && args.source.trim().length > 0 + ? args.source.trim() + : null; + const sourceUrl = + typeof args.source_url === 'string' && args.source_url.trim().length > 0 + ? args.source_url.trim() + : null; + const errs = new ArgErrors(); + rejectRating(errs, args); + if (!title) errs.add('title is required'); + else if (title.length > MAX_RECIPE_TITLE_CHARS) { + errs.add(`title exceeds ${MAX_RECIPE_TITLE_CHARS}-char limit (got ${title.length})`); + } + if (!cooklang) errs.add('cooklang is required'); + else if (cooklang.length > MAX_RECIPE_COOKLANG_CHARS) { + errs.add( + `cooklang exceeds ${MAX_RECIPE_COOKLANG_CHARS}-char limit (got ${cooklang.length})`, + ); + } else { + // Syntax check only on a present, length-legal body - running it on an + // oversize blob just stacks a second complaint about the same field. + const cooklangErrors = validateCooklangSource(cooklang); + if (cooklangErrors.length > 0) { + errs.add(`cooklang validation failed:\n- ${cooklangErrors.join('\n- ')}`); } - if (!title) errs.add('title is required'); - else if (title.length > MAX_RECIPE_TITLE_CHARS) { - errs.add(`title exceeds ${MAX_RECIPE_TITLE_CHARS}-char limit (got ${title.length})`); + } + errs.throwIfAny(); + + // Natural-key dedup: titles are unique per user (recipes_user_title_unique). + // The model may be updating a recipe whose title it knows but whose id it + // forgot; an exact-title match routes to that recipe's update path instead + // of bouncing with the unique-violation, and a near-match is refused with + // the candidates named. No fuzzy-merge risk here beyond the refusal: an + // update only patches named fields, so a wrong guess surfaces as the model + // reading the returned row and correcting itself. + const match = await resolveNaturalKeyMatch({ + query: async (probeValue: string) => { + // % wildcards for containment (see wiki_save.ts); bare column + // select (PostgREST cannot carry AS aliases). RLS OFF: user_id + // filtered explicitly - service-role bypasses RLS. + const { data, error } = await ctx.adminClient + .from('recipes') + .select('id, title') + .eq('user_id', ctx.userId) + .ilike('title', `%${probeValue}%`) + .limit(3); + if (error) throw new Error(`naturalKeyProbe failed: ${error.message}`); + const rows = (data ?? []) as { id: string; title: string }[]; + return { data: rows.map((r) => ({ id: r.id, key: r.title })) }; + }, + keyValue: title, + fuzzy: true, + }); + if (match) { + // The model's create-intent landed on an existing recipe. The edit + // form's change_message requirement would now reject the call for + // a field the model had no reason to provide (the schema calls it + // optional on create), so default it here - the matched-update + // changelog line stays informative. + if ( + typeof args.change_message !== 'string' || + args.change_message.trim().length === 0 + ) { + args = { ...args, change_message: `Matched existing "${match.key}"` }; } - if (!cooklang) errs.add('cooklang is required'); - else if (cooklang.length > MAX_RECIPE_COOKLANG_CHARS) { + const updated = (await doUpdate(match.id, args, ctx)) as { + matched_existing?: boolean; + }; + return { ...updated, matched_existing: true }; + } + // A save is always a recipe's first version, so an omitted + // change_message defaults rather than erroring - there is no prior + // state to describe a delta against, and the model routinely forgets + // the field on a brand-new recipe. The edit form requires it (a + // delta with nothing to describe against is the point of the + // history). Matches the backfill seed naming and the client-side + // recipe_save executor. + const changeMessage = + typeof args.change_message === 'string' && args.change_message.trim().length > 0 + ? args.change_message.trim() + : 'Initial version'; + + const { data, error } = await ctx.adminClient.rpc('recipe_create_with_version', { + p_title: title, + p_cooklang: cooklang, + p_source: source, + p_source_url: sourceUrl, + p_rating: null, + p_image_ids: null, + p_image_labels: null, + p_change_message: changeMessage, + p_user_id: ctx.userId, + }); + if (error) throw new Error(`createRecipe failed: ${error.message}`); + const rows = (data ?? []) as Array<{ + id: string; + title: string; + updated_at: string; + }>; + if (rows.length === 0) throw new Error('createRecipe returned no row'); + const row = rows[0]!; + + return { + id: row.id, + title: row.title, + updated_at: row.updated_at, + photos: [] as Array<{ id: string; position: number }>, + }; +} + +async function doUpdate( + id: string, + args: Record, + ctx: ToolContext, +): Promise { + const errs = new ArgErrors(); + + // Build the RPC arg bundle from the patch shape. The RPC uses + // p_set_ + p_ pairs: explicit null clears, omission + // leaves alone. A malformed field records an error and stays unset, so + // the throw below fires before the RPC ever runs. + let setTitle = false; + let titleVal: string | null = null; + if (typeof args.title === 'string' && args.title.trim().length > 0) { + const t = args.title.trim(); + if (t.length > MAX_RECIPE_TITLE_CHARS) { + errs.add(`title exceeds ${MAX_RECIPE_TITLE_CHARS}-char limit (got ${t.length})`); + } else { + setTitle = true; + titleVal = t; + } + } + + let setCooklang = false; + let cooklangVal: string | null = null; + if (typeof args.cooklang === 'string' && args.cooklang.length > 0) { + if (args.cooklang.length > MAX_RECIPE_COOKLANG_CHARS) { errs.add( - `cooklang exceeds ${MAX_RECIPE_COOKLANG_CHARS}-char limit (got ${cooklang.length})`, + `cooklang exceeds ${MAX_RECIPE_COOKLANG_CHARS}-char limit (got ${args.cooklang.length})`, ); } else { - // Syntax check only on a present, length-legal body - running it on an - // oversize blob just stacks a second complaint about the same field. - const cooklangErrors = validateCooklangSource(cooklang); + const cooklangErrors = validateCooklangSource(args.cooklang); if (cooklangErrors.length > 0) { errs.add(`cooklang validation failed:\n- ${cooklangErrors.join('\n- ')}`); + } else { + setCooklang = true; + cooklangVal = args.cooklang; } } - errs.throwIfAny(); - // A save is always a recipe's first version, so an omitted - // change_message defaults rather than erroring - there is no prior - // state to describe a delta against, and the model routinely forgets - // the field on a brand-new recipe. Matches the backfill seed naming - // and the client-side recipe_save executor. - const changeMessage = - typeof args.change_message === 'string' && args.change_message.trim().length > 0 - ? args.change_message.trim() - : 'Initial version'; - - const { data, error } = await ctx.adminClient.rpc('recipe_create_with_version', { - p_title: title, - p_cooklang: cooklang, - p_source: source, - p_source_url: sourceUrl, - p_rating: null, - p_image_ids: null, - p_image_labels: null, - p_change_message: changeMessage, - p_user_id: ctx.userId, - }); - if (error) throw new Error(`createRecipe failed: ${error.message}`); - const rows = (data ?? []) as Array<{ - id: string; - title: string; - updated_at: string; - }>; - if (rows.length === 0) throw new Error('createRecipe returned no row'); - const row = rows[0]!; - - return { - id: row.id, - title: row.title, - updated_at: row.updated_at, - photos: [] as Array<{ id: string; position: number }>, - }; + } + + let setSource = false; + let sourceVal: string | null = null; + if (args.source === null) { + setSource = true; + sourceVal = null; + } else if (typeof args.source === 'string') { + setSource = true; + sourceVal = args.source.trim(); + } + + let setSourceUrl = false; + let sourceUrlVal: string | null = null; + if (args.source_url === null) { + setSourceUrl = true; + sourceUrlVal = null; + } else if (typeof args.source_url === 'string') { + setSourceUrl = true; + sourceUrlVal = args.source_url.trim(); + } + + rejectRating(errs, args); + + // Empty-patch is only a real complaint when nothing else is wrong - a + // malformed field already left its set-flag false, and double-reporting + // it as "provide at least one of" would mislead. + if (!setTitle && !setCooklang && !setSource && !setSourceUrl && !errs.any) { + errs.add('provide at least one of title, cooklang, source, or source_url'); + } + + const changeMessage = + typeof args.change_message === 'string' ? args.change_message.trim() : ''; + if (!changeMessage) errs.add('change_message is required'); + errs.throwIfAny(); + + const { data, error } = await ctx.adminClient.rpc('recipe_update_with_version', { + p_id: id, + p_set_title: setTitle, + p_title: titleVal, + p_set_cooklang: setCooklang, + p_cooklang: cooklangVal, + p_set_source: setSource, + p_source: sourceVal, + p_set_source_url: setSourceUrl, + p_source_url: sourceUrlVal, + p_set_rating: false, + p_rating: null, + p_set_image_ids: false, + p_image_ids: null, + p_image_labels: null, + p_change_message: changeMessage, + p_user_id: ctx.userId, + }); + if (error) throw new Error(`updateRecipe failed: ${error.message}`); + const rows = (data ?? []) as Array>; + if (rows.length === 0) throw new Error('updateRecipe returned no row'); + + // Drop `topics` from the echoed row. The + // clear_recipe_topics_on_change trigger empties the column on any + // content edit so the recipe-topics curation unit re-tags it, and + // the RPC reads the row back after that trigger has fired - so this + // field is ALWAYS an empty array here, whatever the recipe was + // tagged with a moment earlier and will be tagged with again once + // the unit catches up. Echoing it invited the model to report the + // tags as lost. Callers that want the live tags read them back with + // recipe_get after the curation unit has run. + const { topics: _requeuedTopics, ...row } = rows[0]; + + return { ...row, photos: await readRecipePhotoMeta(ctx.adminClient, id) }; +} + +export const recipeSave: ToolDef = { + name: 'recipe_save', + async execute(args: Record, ctx: ToolContext) { + const id = + typeof args.id === 'string' && args.id.trim().length > 0 + ? args.id.trim() + : undefined; + + // Shared pre-route guard: the rating veto applies to both forms. + if ('rating' in args) { + const errs = new ArgErrors(); + errs.add( + 'rating is not editable by this tool - the star rating is the ' + + "user's own evaluation and only they can set or clear it", + ); + errs.throwIfAny(); + } + + if (id) return doUpdate(id, args, ctx); + return doCreate(args, ctx); }, }; diff --git a/supabase/functions/venice/tools/recipe_update.ts b/supabase/functions/venice/tools/recipe_update.ts deleted file mode 100644 index 880f46b7..00000000 --- a/supabase/functions/venice/tools/recipe_update.ts +++ /dev/null @@ -1,150 +0,0 @@ -// recipe_update (function-side port) -// -// Patch an existing recipe via recipe_update_with_version RPC -// (p_user_id-aware). Cooklang validation mirrors recipe_save's -// inline check. -// -// This tool never sets the photo list (p_set_image_ids=false), so the -// RPC carries the previous version's links onto the new version. The -// response still reads those links back and reports them: an update -// that reported `photos: []` looked like it had wiped the recipe's -// photos, and the model relayed that to the user as data loss on an -// edit that had in fact preserved every photo. Changing the photo set -// is the recipe_photos_* tools' job. - -import { registerTool, type ToolContext, type ToolDef } from '../performToolCall.ts'; -import { readRecipePhotoMeta } from './_recipe_helpers.ts'; -import { ArgErrors } from './_validate.ts'; - -// Mirror of src/lib/recipe-limits.ts - the caps the wire schema -// advertises. Divergent copies here rejected schema-legal bodies. -const MAX_RECIPE_TITLE_CHARS = 160; -const MAX_RECIPE_COOKLANG_CHARS = 20_000; - -import { validateCooklangSource } from '../../_shared/cooklang-validate.ts'; - -export const recipeUpdate: ToolDef = { - name: 'recipe_update', - async execute(args: Record, ctx: ToolContext) { - const id = typeof args.id === 'string' ? args.id : ''; - - const errs = new ArgErrors(); - if (!id) errs.add('id is required'); - - // Build the RPC arg bundle from the patch shape. The RPC uses - // p_set_ + p_ pairs: explicit null clears, omission - // leaves alone. A malformed field records an error and stays unset, so - // the throw below fires before the RPC ever runs. - let setTitle = false; - let titleVal: string | null = null; - if (typeof args.title === 'string' && args.title.trim().length > 0) { - const t = args.title.trim(); - if (t.length > MAX_RECIPE_TITLE_CHARS) { - errs.add(`title exceeds ${MAX_RECIPE_TITLE_CHARS}-char limit (got ${t.length})`); - } else { - setTitle = true; - titleVal = t; - } - } - - let setCooklang = false; - let cooklangVal: string | null = null; - if (typeof args.cooklang === 'string' && args.cooklang.length > 0) { - if (args.cooklang.length > MAX_RECIPE_COOKLANG_CHARS) { - errs.add( - `cooklang exceeds ${MAX_RECIPE_COOKLANG_CHARS}-char limit (got ${args.cooklang.length})`, - ); - } else { - const cooklangErrors = validateCooklangSource(args.cooklang); - if (cooklangErrors.length > 0) { - errs.add(`cooklang validation failed:\n- ${cooklangErrors.join('\n- ')}`); - } else { - setCooklang = true; - cooklangVal = args.cooklang; - } - } - } - - let setSource = false; - let sourceVal: string | null = null; - if (args.source === null) { - setSource = true; - sourceVal = null; - } else if (typeof args.source === 'string') { - setSource = true; - sourceVal = args.source.trim(); - } - - let setSourceUrl = false; - let sourceUrlVal: string | null = null; - if (args.source_url === null) { - setSourceUrl = true; - sourceUrlVal = null; - } else if (typeof args.source_url === 'string') { - setSourceUrl = true; - sourceUrlVal = args.source_url.trim(); - } - - // The star rating is the user's evaluation of a recipe they cooked, - // not recipe content. The model would set it from conversational - // praise ("that turned out great") and overwrite a verdict only the - // user gets to make, so the tool refuses it outright rather than - // ignoring it silently - a silent drop reads to the model as a - // successful write and it tells the user the rating changed. - if ('rating' in args) { - errs.add( - 'rating is not editable by this tool - the star rating is the ' + - "user's own evaluation and only they can set or clear it", - ); - } - - // Empty-patch is only a real complaint when nothing else is wrong - a - // malformed field already left its set-flag false, and double-reporting - // it as "provide at least one of" would mislead. - if (!setTitle && !setCooklang && !setSource && !setSourceUrl && !errs.any) { - errs.add('provide at least one of title, cooklang, source, or source_url'); - } - - const changeMessage = - typeof args.change_message === 'string' ? args.change_message.trim() : ''; - if (!changeMessage) errs.add('change_message is required'); - errs.throwIfAny(); - - const { data, error } = await ctx.adminClient.rpc('recipe_update_with_version', { - p_id: id, - p_set_title: setTitle, - p_title: titleVal, - p_set_cooklang: setCooklang, - p_cooklang: cooklangVal, - p_set_source: setSource, - p_source: sourceVal, - p_set_source_url: setSourceUrl, - p_source_url: sourceUrlVal, - p_set_rating: false, - p_rating: null, - p_set_image_ids: false, - p_image_ids: null, - p_image_labels: null, - p_change_message: changeMessage, - p_user_id: ctx.userId, - }); - if (error) throw new Error(`updateRecipe failed: ${error.message}`); - const rows = (data ?? []) as Array>; - if (rows.length === 0) throw new Error('updateRecipe returned no row'); - - // Drop `topics` from the echoed row. The - // clear_recipe_topics_on_change trigger empties the column on any - // content edit so the recipe-topics curation unit re-tags it, and - // the RPC reads the row back after that trigger has fired - so this - // field is ALWAYS an empty array here, whatever the recipe was - // tagged with a moment earlier and will be tagged with again once - // the unit catches up. Echoing it invited the model to report the - // tags as lost. Callers that want the live tags read them back with - // recipe_get after the curation unit has run. - const { topics: _requeuedTopics, ...row } = rows[0]; - - return { ...row, photos: await readRecipePhotoMeta(ctx.adminClient, id) }; - }, -}; - -registerTool(recipeUpdate); diff --git a/supabase/schema.sql b/supabase/schema.sql index 66ad0570..45f2fbd0 100644 --- a/supabase/schema.sql +++ b/supabase/schema.sql @@ -2552,8 +2552,7 @@ create table if not exists public.recipes ( title text not null, source text, source_url text, - cooklang text not null, - -- User rating, 1-5 stars. Null means "unrated"; clearing the stars in + cooklang text not null, -- User rating, 1-5 stars. Null means "unrated"; clearing the stars in -- the UI writes null rather than 0 so the unrated case is -- distinguishable from "actively rated zero" (which we don't allow). rating smallint, @@ -2583,6 +2582,22 @@ do $$ begin end if; end $$; +-- (user_id, lower(trim(title))) is unique: recipe titles are the +-- Cookbook's display and lookup key, and duplicates read as bugs - two +-- "Weeknight Chili" cards that drift apart as one gets edited. The +-- lower(trim(..)) shape matches how the recipe_save tool's dedup +-- heuristic probes, so an exact-title create lands as a visible +-- constraint error only when the probe raced; the heuristic is the +-- primary guard. Enforced as a unique INDEX rather than a table +-- constraint: Postgres only accepts expressions in index form +-- (`unique (col, (expr))` in a table constraint is a syntax error). +-- Prod was verified collision-free (50 rows, 0 ci-collisions, 0 +-- untrimmed) before this landed; a violating table would need a +-- manual rename pass first, which the deploy-time failure would +-- surface loudly. +create unique index if not exists recipes_user_title_unique + on public.recipes (user_id, lower(btrim(title))); + create index if not exists recipes_user_updated_idx on public.recipes (user_id, updated_at desc); @@ -3157,11 +3172,28 @@ begin raise exception 'image_labels length must match image_ids length'; end if; - insert into public.recipes (user_id, title, source, source_url, cooklang, - rating, created_at, updated_at) - values (v_uid, p_title, p_source, p_source_url, p_cooklang, - p_rating, v_now, v_now) - returning recipes.id into v_recipe_id; + -- The (user_id, lower(trim(title))) unique constraint is the + -- load-bearing dedup guard. Catch the 23505 here so the agent-facing + -- error names the remedy (search then save by id) instead of the + -- raw "duplicate key" text, which reads as a transient failure. + -- The recipe_save tool's natural-key heuristic catches the common + -- case first; this is the race fallback. Attached to a nested + -- BEGIN..END block: plpgsql only honors an exception clause at the + -- end of a block, and a bare `exception` mid-body would silently + -- swallow every statement after it (empirically verified - the + -- version insert never ran with the handler sitting mid-block). + begin + insert into public.recipes (user_id, title, source, source_url, cooklang, + rating, created_at, updated_at) + values (v_uid, p_title, p_source, p_source_url, p_cooklang, + p_rating, v_now, v_now) + returning recipes.id into v_recipe_id; + exception + when unique_violation then + raise exception + 'a recipe titled "%" already exists - run recipe_list to find its id, then call recipe_save with that id to update it', + p_title; + end; insert into public.recipe_versions (recipe_id, user_id, title, source, source_url, cooklang, rating, @@ -3318,12 +3350,23 @@ begin -- absent leaves it alone; explicit null clears (back to "unrated"). if p_set_rating then v_rating := p_rating; end if; - update public.recipes - set title = v_title, cooklang = v_cooklang, - source = v_source, source_url = v_source_url, - rating = v_rating, - updated_at = v_now - where recipes.id = p_id; + -- A rename can collide with another recipe's title (see the + -- recipes_user_title_unique constraint). Catch the 23505 so the + -- error names the situation instead of raw "duplicate key" text. + -- Nested BEGIN..END for the same reason as the create RPC's handler. + begin + update public.recipes + set title = v_title, cooklang = v_cooklang, + source = v_source, source_url = v_source_url, + rating = v_rating, + updated_at = v_now + where recipes.id = p_id; + exception + when unique_violation then + raise exception + 'another recipe is already titled "%" - pick a different name, or update that recipe instead', + v_title; + end; insert into public.recipe_versions (recipe_id, user_id, title, source, source_url, cooklang, rating, diff --git a/tests/system-prompt.test.ts b/tests/system-prompt.test.ts index c6e75433..877af48a 100644 --- a/tests/system-prompt.test.ts +++ b/tests/system-prompt.test.ts @@ -408,7 +408,7 @@ describe('every tool available every turn', () => { ]) { expect(prompt).toMatch(new RegExp(`^ ${name} : `, 'm')); } - expect(prompt).toMatch(/^ - recipe_update : /m); + expect(prompt).toMatch(/^ - recipe_save : /m); expect(prompt).toMatch(/^ - memory_search : /m); }); diff --git a/tests/tools.test.ts b/tests/tools.test.ts index 156007ed..96dea5b8 100644 --- a/tests/tools.test.ts +++ b/tests/tools.test.ts @@ -39,7 +39,6 @@ describe('tool registry', () => { // a tool here would be a silent failure the model cannot see. const names = buildToolList([]).map((t) => t.function.name).sort(); expect(names).toEqual(TOOLS.map((t: ToolDef) => t.name).sort()); - expect(names).toContain('recipe_update'); expect(names).toContain('memory_save'); }); @@ -89,7 +88,6 @@ describe('tool registry', () => { // carry only the tools that mutate user data. expect(cookingToolbox.tools.map((t: ToolDef) => t.name)).toEqual([ 'recipe_save', - 'recipe_update', 'recipe_delete', 'recipe_photos_attach', 'recipe_photos_remove',