From eeee9843941aa5afa4144312127bc5704ff9f3a9 Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Sat, 22 Aug 2026 01:55:57 +0000 Subject: [PATCH] give FableLoom paths their own routes so one writer can't clobber another (#4786) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adding one intent transition meant PATCHing the node with the whole `transitions` array, so a second writer working from a stale snapshot dropped every row it had never seen — and the editor had to reconcile server-minted ids back into its locally-added rows after each save. Transitions are now sub-resources under their scene, mirroring the node routes: POST mints the `tr-*` id and answers `{ loom, transition }`, PATCH and DELETE address one edge by id and answer with the loom. The whole-array key on the node PATCH keeps working unchanged, so a client or peer from before these routes is unaffected. The scene editor creates a path server-side first, so every row it holds already carries its id; blur-saves, the target select, and remove each touch exactly one edge, and the id re-sync workaround is gone. --- .../components/fableloom/LoomNodeEditor.jsx | 118 +++++++++++------- .../fableloom/LoomNodeEditor.test.jsx | 107 ++++++++++++++++ client/src/services/README.md | 2 +- client/src/services/apiFableLoom.js | 19 +++ client/src/services/apiFableLoom.test.js | 22 ++++ docs/features/fableloom.md | 17 +++ server/lib/README.md | 2 +- server/lib/fableLoomValidation.js | 24 +++- server/routes/fableLoom.js | 32 +++++ server/routes/fableLoom.test.js | 58 +++++++++ server/services/fableLoom/README.md | 2 +- server/services/fableLoom/index.js | 4 + server/services/fableLoom/records.js | 68 ++++++++++ server/services/fableLoom/records.test.js | 80 +++++++++++- 14 files changed, 500 insertions(+), 55 deletions(-) create mode 100644 client/src/components/fableloom/LoomNodeEditor.test.jsx diff --git a/client/src/components/fableloom/LoomNodeEditor.jsx b/client/src/components/fableloom/LoomNodeEditor.jsx index 18c0da851..b6d5187a6 100644 --- a/client/src/components/fableloom/LoomNodeEditor.jsx +++ b/client/src/components/fableloom/LoomNodeEditor.jsx @@ -4,9 +4,11 @@ * queued render via the shared image-gen lane), and the AI branch action. * * Fields save on blur (silent PATCH, skipped when unchanged; the server - * returns the full loom, which the parent folds into state). The AI actions - * read server-side state, so they gate on in-flight saves per the client - * save-gating convention. + * returns the full loom, which the parent folds into state). Paths save one + * row at a time against the transition sub-resources — a row exists on the + * server the moment it is added, so its id is known here and nothing has to be + * reconciled back after a save. The AI actions read server-side state, so they + * gate on in-flight saves per the client save-gating convention. */ import { useEffect, useMemo, useState } from 'react'; @@ -18,25 +20,26 @@ import MediaImage from '../MediaImage'; import { useAsyncAction } from '../../hooks/useAsyncAction'; import { useConfirmDelete } from '../../hooks/useConfirmDelete'; import { - branchLoomNode, deleteLoomNode, generateImage, updateLoomNode, + addLoomTransition, branchLoomNode, deleteLoomNode, deleteLoomTransition, + generateImage, updateLoomNode, updateLoomTransition, } from '../../services/api'; import { fieldClass, labelClass, sceneFieldClass } from './fieldStyles'; import { isTeleplayFormat } from './loomFormats'; const toRow = (t) => ({ ...t, triggersText: (t.triggers || []).join('; ') }); -const rowsToTransitions = (rows) => rows - .filter((t) => t.targetNodeId) - .map(({ id, targetNodeId, intent, triggersText, description }) => ({ - id, targetNodeId, intent, - triggers: (triggersText || '').split(';').map((s) => s.trim()).filter(Boolean), - description: description || '', - })); +const rowToPatch = ({ targetNodeId, intent, triggersText, description }) => ({ + targetNodeId, + intent: intent || '', + triggers: (triggersText || '').split(';').map((s) => s.trim()).filter(Boolean), + description: description || '', +}); export default function LoomNodeEditor({ loom, episode, node, onLoomUpdate, onClearSelection, onMakeStart }) { const [form, setForm] = useState(null); // In-flight blur-saves; the AI buttons (which read server-side state) stay // disabled until every pending save settles. const [pendingSaves, setPendingSaves] = useState(0); + const [addingPath, setAddingPath] = useState(false); const del = useConfirmDelete(); // A teleplay carries its own line breaks, so the editor gives it a taller // monospaced field — the same surface prose gets, sized for the format. @@ -63,11 +66,17 @@ export default function LoomNodeEditor({ loom, episode, node, onLoomUpdate, onCl [episode.nodes, node.id], ); - const patchNode = async (patch) => { + // Every write from this panel goes through here so the AI gate sees it and a + // failure surfaces once, in one place. + const runSave = async (write) => { setPendingSaves((n) => n + 1); - const updated = await updateLoomNode(loom.id, episode.id, node.id, patch, { silent: true }) - .catch((err) => { toast.error(`Save failed: ${err.message}`); return null; }); + const result = await write().catch((err) => { toast.error(`Save failed: ${err.message}`); return null; }); setPendingSaves((n) => n - 1); + return result; + }; + + const patchNode = async (patch) => { + const updated = await runSave(() => updateLoomNode(loom.id, episode.id, node.id, patch, { silent: true })); if (updated) onLoomUpdate(updated); return updated; }; @@ -79,23 +88,18 @@ export default function LoomNodeEditor({ loom, episode, node, onLoomUpdate, onCl return patchNode({ [key]: value }); }; - const syncTransitionsFrom = (updatedLoom) => { - const saved = updatedLoom?.episodes.find((e) => e.id === episode.id) - ?.nodes.find((n) => n.id === node.id)?.transitions; - if (!saved) return; - setForm((prev) => ({ ...prev, transitions: saved.map(toRow) })); - }; - - const saveTransitions = async (rows) => { - const payload = rowsToTransitions(rows); - const current = (node.transitions || []).map((t) => ({ - id: t.id, targetNodeId: t.targetNodeId, intent: t.intent, triggers: t.triggers, description: t.description, - })); - if (JSON.stringify(payload) === JSON.stringify(current)) return; - const updated = await patchNode({ transitions: payload }); - // Re-sync just the transition rows so server-minted ids replace the - // locally-added rows' missing ones (id churn otherwise re-mints per save). - syncTransitionsFrom(updated); + // Blur-save for one path. Skipped when the row already matches the record, + // so tabbing through a path doesn't rewrite the loom. + const saveTransition = async (row) => { + const saved = (node.transitions || []).find((t) => t.id === row.id); + const patch = rowToPatch(row); + // No record row to compare against means the panel is ahead of the loom in + // state, NOT that nothing changed — save rather than silently drop the edit. + if (saved && JSON.stringify(patch) === JSON.stringify(rowToPatch(toRow(saved)))) return; + const updated = await runSave( + () => updateLoomTransition(loom.id, episode.id, node.id, row.id, patch, { silent: true }), + ); + if (updated) onLoomUpdate(updated); }; // The save fires OUTSIDE the setState updater — StrictMode runs updaters @@ -103,31 +107,46 @@ export default function LoomNodeEditor({ loom, episode, node, onLoomUpdate, onCl const applyTransition = (index, patch, { save = false } = {}) => { const transitions = form.transitions.map((t, i) => (i === index ? { ...t, ...patch } : t)); setForm((prev) => ({ ...prev, transitions })); - if (save) saveTransitions(transitions); + if (save) saveTransition(transitions[index]); }; - const removeTransition = (index) => { - const transitions = form.transitions.filter((_, i) => i !== index); - setForm((prev) => ({ ...prev, transitions })); - saveTransitions(transitions); + const removeTransition = async (row) => { + setForm((prev) => ({ ...prev, transitions: prev.transitions.filter((t) => t.id !== row.id) })); + const updated = await runSave( + () => deleteLoomTransition(loom.id, episode.id, node.id, row.id, { silent: true }), + ); + // The row went out of the list before the round-trip; put the record back + // if the delete never landed, rather than leaving a path that only looks gone. + if (updated) onLoomUpdate(updated); + else setForm((prev) => ({ ...prev, transitions: (node.transitions || []).map(toRow) })); }; - const addTransition = () => { + // The row is created server-side first, so it arrives with its id already + // set and every later edit is a plain PATCH against it. + const addTransition = async () => { const target = otherNodes[0]; if (!target) { toast.error('Add another scene first — a path needs somewhere to go'); return; } - setForm((prev) => ({ - ...prev, - transitions: [...prev.transitions, { targetNodeId: target.id, intent: '', triggersText: '', description: '' }], - })); + setAddingPath(true); + const result = await runSave( + () => addLoomTransition(loom.id, episode.id, node.id, { targetNodeId: target.id, intent: '' }, { silent: true }), + ); + setAddingPath(false); + if (!result?.transition) return; + setForm((prev) => ({ ...prev, transitions: [...prev.transitions, toRow(result.transition)] })); + onLoomUpdate(result.loom); }; const [runBranch, branching] = useAsyncAction(async () => { const result = await branchLoomNode(loom.id, episode.id, node.id, { branchCount: 2 }, { silent: true }); onLoomUpdate(result.loom); - syncTransitionsFrom(result.loom); + // The AI writes new paths straight onto the record; this panel is keyed by + // node.id so it never remounts to pick them up. + const woven = result.loom?.episodes.find((e) => e.id === episode.id) + ?.nodes.find((n) => n.id === node.id)?.transitions; + if (woven) setForm((prev) => ({ ...prev, transitions: woven.map(toRow) })); toast.success('New branches woven'); }, { errorMessage: 'Branching failed' }); @@ -280,7 +299,12 @@ export default function LoomNodeEditor({ loom, episode, node, onLoomUpdate, onCl {branching ? : } Branch with AI - @@ -290,7 +314,7 @@ export default function LoomNodeEditor({ loom, episode, node, onLoomUpdate, onCl )}
{form.transitions.map((tr, index) => ( -
+
applyTransition(index, { intent: e.target.value })} - onBlur={() => saveTransitions(form.transitions)} + onBlur={() => saveTransition(tr)} />
))} diff --git a/client/src/components/fableloom/LoomNodeEditor.test.jsx b/client/src/components/fableloom/LoomNodeEditor.test.jsx new file mode 100644 index 000000000..a6a99cb0e --- /dev/null +++ b/client/src/components/fableloom/LoomNodeEditor.test.jsx @@ -0,0 +1,107 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +vi.mock('../../services/api', () => ({ + addLoomTransition: vi.fn(), + branchLoomNode: vi.fn(), + deleteLoomNode: vi.fn(), + deleteLoomTransition: vi.fn(), + generateImage: vi.fn(), + updateLoomNode: vi.fn(), + updateLoomTransition: vi.fn(), +})); +vi.mock('../MediaImage', () => ({ default: () => null })); + +import { + addLoomTransition, deleteLoomTransition, updateLoomNode, updateLoomTransition, +} from '../../services/api'; +import LoomNodeEditor from './LoomNodeEditor'; + +const loom = { id: 'loom-1', name: 'Example Story', format: 'prose', styleNotes: '' }; + +// One scene with a single existing path, plus a second scene to point at. +const makeNodes = (transitions) => ([ + { id: 'n1', title: 'The Gate', prose: 'You stand before it.', transitions }, + { id: 'n2', title: 'Inside', prose: 'Torchlight.', transitions: [] }, +]); + +const existingPath = { id: 'tr-1', targetNodeId: 'n2', intent: 'enter', triggers: ['go in'], description: '' }; + +const renderEditor = (transitions = [existingPath]) => { + const nodes = makeNodes(transitions); + const episode = { id: 'ep-1', startNodeId: 'n1', nodes }; + const onLoomUpdate = vi.fn(); + render( + {}} + />, + ); + return { onLoomUpdate }; +}; + +beforeEach(() => vi.clearAllMocks()); + +describe('LoomNodeEditor paths', () => { + it('creates a path server-side first, so the new row already carries its id', async () => { + const user = userEvent.setup(); + const minted = { id: 'tr-9', targetNodeId: 'n2', intent: '', triggers: [], description: '' }; + addLoomTransition.mockResolvedValue({ loom: { id: 'loom-1' }, transition: minted }); + const { onLoomUpdate } = renderEditor([]); + + await user.click(screen.getByRole('button', { name: '+ Add path' })); + + await waitFor(() => expect(addLoomTransition).toHaveBeenCalledTimes(1)); + expect(addLoomTransition).toHaveBeenCalledWith( + 'loom-1', 'ep-1', 'n1', { targetNodeId: 'n2', intent: '' }, { silent: true }, + ); + expect(onLoomUpdate).toHaveBeenCalledWith({ id: 'loom-1' }); + await waitFor(() => expect(screen.getByText('Paths out (1)')).toBeInTheDocument()); + // The whole-array node PATCH is not how a path is added any more. + expect(updateLoomNode).not.toHaveBeenCalled(); + }); + + it('saves one edited row by id rather than replaying the array', async () => { + const user = userEvent.setup(); + updateLoomTransition.mockResolvedValue({ id: 'loom-1' }); + renderEditor(); + + const intent = screen.getByLabelText('Intent'); + await user.clear(intent); + await user.type(intent, 'slip past'); + await user.tab(); + + await waitFor(() => expect(updateLoomTransition).toHaveBeenCalledTimes(1)); + expect(updateLoomTransition).toHaveBeenCalledWith('loom-1', 'ep-1', 'n1', 'tr-1', { + targetNodeId: 'n2', intent: 'slip past', triggers: ['go in'], description: '', + }, { silent: true }); + expect(updateLoomNode).not.toHaveBeenCalled(); + }); + + it('skips the round-trip when a blurred row still matches the record', async () => { + const user = userEvent.setup(); + renderEditor(); + + await user.click(screen.getByLabelText('Intent')); + await user.tab(); + + expect(updateLoomTransition).not.toHaveBeenCalled(); + }); + + it('deletes one path by id', async () => { + const user = userEvent.setup(); + deleteLoomTransition.mockResolvedValue({ id: 'loom-1' }); + const { onLoomUpdate } = renderEditor(); + + await user.click(screen.getByRole('button', { name: 'Remove path' })); + + await waitFor(() => expect(deleteLoomTransition).toHaveBeenCalledTimes(1)); + expect(deleteLoomTransition).toHaveBeenCalledWith('loom-1', 'ep-1', 'n1', 'tr-1', { silent: true }); + expect(onLoomUpdate).toHaveBeenCalledWith({ id: 'loom-1' }); + expect(screen.getByText('Paths out (0)')).toBeInTheDocument(); + }); +}); diff --git a/client/src/services/README.md b/client/src/services/README.md index bcdecffbd..2a640b278 100644 --- a/client/src/services/README.md +++ b/client/src/services/README.md @@ -102,7 +102,7 @@ toasts on throw). **Custom catch ⇒ `silent: true`** — otherwise toasts fire | `apiMediaJobs.js` | Media generation job tracking + `refineMediaPrompt` / `promptFromMedia` (vision reverse-prompt). | | `apiCreativeDirector.js` | Creative Director (video production). | | `apiCreativeCommission.js` | Creative Commissions (Autonomous Creation Engine — standing recurring briefs). | -| `apiFableLoom.js` | FableLoom branching narratives — loom/episode/scene-node CRUD, deterministic graph validation, and the AI lanes (weave, branch, review, play turns). | +| `apiFableLoom.js` | FableLoom branching narratives — loom/episode/scene-node/transition CRUD, deterministic graph validation, and the AI lanes (weave, branch, review, play turns). | | `apiGames.js` | Game studio records, managed-app binding, reusable sprite/music bindings, deterministic asset-bundle compilation/integrity preflight, and AI feedback history. | | `apiMusicVideo.js` | Music Video projects + scene board + audio analysis. | | `apiSprites.js` | Sprite Manager records, asset library, production-set import (#2895), reference workflow: create/generate/lock (#2896), directional walk and per-track generation/approval, animation-type definition CRUD (#3153), trim/postprocess, and per-run source-frame listing for the Loop Trimmer's re-derive (#2980). | diff --git a/client/src/services/apiFableLoom.js b/client/src/services/apiFableLoom.js index 4fa8d0dfb..7a3ef2473 100644 --- a/client/src/services/apiFableLoom.js +++ b/client/src/services/apiFableLoom.js @@ -5,6 +5,8 @@ const episodePath = (id, episodeId, rest = '') => loomPath(id, `/episodes/${encodeURIComponent(episodeId)}${rest}`); const nodePath = (id, episodeId, nodeId, rest = '') => episodePath(id, episodeId, `/nodes/${encodeURIComponent(nodeId)}${rest}`); +const transitionPath = (id, episodeId, nodeId, transitionId) => + nodePath(id, episodeId, nodeId, `/transitions/${encodeURIComponent(transitionId)}`); export const listLooms = (options = {}) => request('/fableloom', options); export const getLoom = (id, options = {}) => request(loomPath(id), options); @@ -41,6 +43,23 @@ export const deleteLoomNode = (id, episodeId, nodeId, options = {}) => request(n method: 'DELETE', ...options, }); +// One path out of a scene per call. `addLoomTransition` resolves to +// `{ loom, transition }` — the row carries its server-minted id, so the editor +// never has to reconcile ids back into locally-added rows. The node PATCH's +// whole-array `transitions` key still works for bulk replaces. +export const addLoomTransition = (id, episodeId, nodeId, body, options = {}) => + request(nodePath(id, episodeId, nodeId, '/transitions'), { + method: 'POST', body: JSON.stringify(body), ...options, + }); +export const updateLoomTransition = (id, episodeId, nodeId, transitionId, patch, options = {}) => + request(transitionPath(id, episodeId, nodeId, transitionId), { + method: 'PATCH', body: JSON.stringify(patch), ...options, + }); +export const deleteLoomTransition = (id, episodeId, nodeId, transitionId, options = {}) => + request(transitionPath(id, episodeId, nodeId, transitionId), { + method: 'DELETE', ...options, + }); + export const weaveLoomEpisode = (id, episodeId, body = {}, options = {}) => request(episodePath(id, episodeId, '/weave'), { method: 'POST', body: JSON.stringify(body), ...options, }); diff --git a/client/src/services/apiFableLoom.test.js b/client/src/services/apiFableLoom.test.js index 7230250bf..c22ef288d 100644 --- a/client/src/services/apiFableLoom.test.js +++ b/client/src/services/apiFableLoom.test.js @@ -47,6 +47,28 @@ describe('apiFableLoom', () => { expect(request).toHaveBeenCalledWith('/fableloom/loom-1/episodes/ep-1/validate', { silent: true }); }); + it('posts a new path to the node transitions sub-resource', async () => { + await api.addLoomTransition('loom-1', 'ep-1', 'node-1', { targetNodeId: 'node-2', intent: '' }, { silent: true }); + expect(request).toHaveBeenCalledWith('/fableloom/loom-1/episodes/ep-1/nodes/node-1/transitions', { + method: 'POST', + body: JSON.stringify({ targetNodeId: 'node-2', intent: '' }), + silent: true, + }); + }); + + it('patches and deletes one path by id, encoding every segment', async () => { + await api.updateLoomTransition('loom-1', 'ep-1', 'node-1', 'tr/1', { intent: 'press on' }); + expect(request).toHaveBeenCalledWith('/fableloom/loom-1/episodes/ep-1/nodes/node-1/transitions/tr%2F1', { + method: 'PATCH', + body: JSON.stringify({ intent: 'press on' }), + }); + + await api.deleteLoomTransition('loom-1', 'ep-1', 'node-1', 'tr-1'); + expect(request).toHaveBeenCalledWith('/fableloom/loom-1/episodes/ep-1/nodes/node-1/transitions/tr-1', { + method: 'DELETE', + }); + }); + it('deletes nodes with DELETE', async () => { await api.deleteLoomNode('loom-1', 'ep-1', 'node-1'); expect(request).toHaveBeenCalledWith('/fableloom/loom-1/episodes/ep-1/nodes/node-1', { method: 'DELETE' }); diff --git a/docs/features/fableloom.md b/docs/features/fableloom.md index c9eef079f..1f6a3bf37 100644 --- a/docs/features/fableloom.md +++ b/docs/features/fableloom.md @@ -32,6 +32,23 @@ answers in-world without leaving the scene when nothing matches. - **Story settings drawer** — scene format (plus the rewrite pass), and the narrator's provider/model/effort pin. +## Editing paths + +A transition is its own sub-resource under its scene, so any writer — the +editor rail, a voice action, a CoS agent — adds or edits ONE edge per call +and never replays the array off a snapshot another writer has already moved: + +| Route | Answers with | +|---|---| +| `POST /api/fableloom/:id/episodes/:episodeId/nodes/:nodeId/transitions` | `{ loom, transition }` — the server mints the `tr-*` id | +| `PATCH …/transitions/:transitionId` | the loom | +| `DELETE …/transitions/:transitionId` | the loom | + +The node PATCH still accepts a whole `transitions` array for a bulk replace +(unchanged, and what a client from before these routes uses). The editor rail +creates a path server-side first, so every row it holds already carries its +id and nothing has to be reconciled back after a save. + ## Scene format A loom is written either as **narrated prose** (second-person interactive diff --git a/server/lib/README.md b/server/lib/README.md index b95133c0d..53a8a74d7 100644 --- a/server/lib/README.md +++ b/server/lib/README.md @@ -45,7 +45,7 @@ The barrel `server/lib/index.js` is a machine-checkable enumeration of every pub | `creativeCommissionValidation.js` | Creative Commission (Autonomous Creation Engine) create/update + brief/schedule/generation schemas. Brief field caps are mirrored by the commission form in `client/src/components/creative-commission/commissionForm.js` (parity: `creativeCommissionValidation.mirror.test.js`). | | `creativeDirectorValidation.js` | Creative Director project/treatment/scene + Create-Suite importer schemas. `CREATIVE_DIRECTOR_GOAL_MAX` is mirrored in `client/src/lib/creativeDirectorPlan.js` (parity: `creativeDirectorValidation.mirror.test.js`). | | `digitalTwinValidation.js` | Digital twin document/category schemas. | -| `fableLoomValidation.js` | FableLoom branching-narrative route schemas (loom/episode/node CRUD, weave/branch/review, play turns). | +| `fableLoomValidation.js` | FableLoom branching-narrative route schemas (loom/episode/node/transition CRUD, weave/branch/review, play turns). | | `genomeValidation.js` | Genome upload + search schemas. | | `identityValidation.js` | Identity section + chronotype + scheduling schemas. | | `meatspaceValidation.js` | Meatspace (location/health log) schemas. | diff --git a/server/lib/fableLoomValidation.js b/server/lib/fableLoomValidation.js index 321692b71..b89a1d2a4 100644 --- a/server/lib/fableLoomValidation.js +++ b/server/lib/fableLoomValidation.js @@ -65,12 +65,32 @@ export const episodePatchSchema = z.object({ startNodeId: nodeIdStr.nullable().optional(), }); -const transitionSchema = z.object({ - id: z.string().max(80).optional(), +const transitionFields = { targetNodeId: nodeIdStr, intent: z.string().max(LOOM_LIMITS.INTENT_MAX), triggers: z.array(z.string().max(LOOM_LIMITS.TRIGGER_MAX)).max(LOOM_LIMITS.TRIGGERS_MAX).optional(), description: z.string().max(LOOM_LIMITS.TRANSITION_DESC_MAX).optional(), +}; + +// Whole-array replace on the node PATCH. Kept for back-compat with clients +// that predate the transition sub-resources (`id` is echoed back so a replace +// preserves the rows it did not change); new writers use the sub-resources. +const transitionSchema = z.object({ + id: z.string().max(80).optional(), + ...transitionFields, +}); + +// Sub-resource POST: no `id` — the server mints it. +export const transitionCreateSchema = z.object(transitionFields); + +// Sub-resource PATCH: every field optional, but `intent` may be cleared to '' +// (a path can legitimately carry only trigger phrasings), so `.optional()` +// rather than a min length is what distinguishes absent from cleared. +export const transitionPatchSchema = z.object({ + targetNodeId: nodeIdStr.optional(), + intent: transitionFields.intent.optional(), + triggers: transitionFields.triggers, + description: transitionFields.description, }); const nodeFields = { diff --git a/server/routes/fableLoom.js b/server/routes/fableLoom.js index aec3cc556..0f9204a57 100644 --- a/server/routes/fableLoom.js +++ b/server/routes/fableLoom.js @@ -22,17 +22,21 @@ import { playTurnSchema, reformatSchema, reviewSchema, + transitionCreateSchema, + transitionPatchSchema, weaveSchema, } from '../lib/fableLoomValidation.js'; import { analyzeEpisodeGraph } from '../lib/fableLoomGraph.js'; import { addEpisode, addNode, + addNodeTransition, branchNode, createLoom, deleteEpisode, deleteLoom, deleteNode, + deleteNodeTransition, getLoom, listLoomSummaries, playTurn, @@ -41,6 +45,7 @@ import { updateEpisode, updateLoom, updateNode, + updateNodeTransition, weaveEpisode, } from '../services/fableLoom/index.js'; @@ -115,6 +120,33 @@ router.delete('/:id/episodes/:episodeId/nodes/:nodeId', asyncHandler(async (req, res.json(await deleteNode(req.params.id, req.params.episodeId, req.params.nodeId)); })); +// --- Transitions ------------------------------------------------------------ +// +// One edge per request. The node PATCH still accepts a whole `transitions` +// array (unchanged, for clients that predate these routes) — but replaying the +// array to add one path means a second writer working off a stale snapshot +// drops the rows it never saw. POST answers with `{ loom, transition }` so the +// caller has the minted id without diffing the array; PATCH/DELETE answer with +// the loom, same as the node routes one level up. + +router.post('/:id/episodes/:episodeId/nodes/:nodeId/transitions', asyncHandler(async (req, res) => { + const input = validateRequest(transitionCreateSchema, req.body); + res.status(201).json(await addNodeTransition(req.params.id, req.params.episodeId, req.params.nodeId, input)); +})); + +router.patch('/:id/episodes/:episodeId/nodes/:nodeId/transitions/:transitionId', asyncHandler(async (req, res) => { + const patch = validateRequest(transitionPatchSchema, req.body); + res.json(await updateNodeTransition( + req.params.id, req.params.episodeId, req.params.nodeId, req.params.transitionId, patch, + )); +})); + +router.delete('/:id/episodes/:episodeId/nodes/:nodeId/transitions/:transitionId', asyncHandler(async (req, res) => { + res.json(await deleteNodeTransition( + req.params.id, req.params.episodeId, req.params.nodeId, req.params.transitionId, + )); +})); + // --- AI lanes --------------------------------------------------------------- router.post('/:id/episodes/:episodeId/weave', asyncHandler(async (req, res) => { diff --git a/server/routes/fableLoom.test.js b/server/routes/fableLoom.test.js index 7669ba370..c2c12f938 100644 --- a/server/routes/fableLoom.test.js +++ b/server/routes/fableLoom.test.js @@ -6,11 +6,13 @@ import { request } from '../lib/testHelper.js'; vi.mock('../services/fableLoom/index.js', () => ({ addEpisode: vi.fn(), addNode: vi.fn(), + addNodeTransition: vi.fn(), branchNode: vi.fn(), createLoom: vi.fn(), deleteEpisode: vi.fn(), deleteLoom: vi.fn(), deleteNode: vi.fn(), + deleteNodeTransition: vi.fn(), getLoom: vi.fn(), listLoomSummaries: vi.fn(async () => []), playTurn: vi.fn(), @@ -19,6 +21,7 @@ vi.mock('../services/fableLoom/index.js', () => ({ updateEpisode: vi.fn(), updateLoom: vi.fn(), updateNode: vi.fn(), + updateNodeTransition: vi.fn(), weaveEpisode: vi.fn(), })); @@ -89,6 +92,61 @@ describe('FableLoom routes', () => { expect(fableLoom.deleteNode).toHaveBeenCalledWith('loom-1', 'ep-1', 'node-1'); }); + it('POST transitions mints one path and answers with the loom plus the row', async () => { + fableLoom.addNodeTransition.mockResolvedValueOnce({ + loom: { id: 'loom-1' }, transition: { id: 'tr-1', targetNodeId: 'node-2', intent: 'press on' }, + }); + const created = await request(makeApp()) + .post('/api/fableloom/loom-1/episodes/ep-1/nodes/node-1/transitions') + .send({ targetNodeId: 'node-2', intent: 'press on', triggers: ['keep going'] }); + expect(created.status).toBe(201); + expect(created.body.transition.id).toBe('tr-1'); + expect(fableLoom.addNodeTransition).toHaveBeenCalledWith('loom-1', 'ep-1', 'node-1', { + targetNodeId: 'node-2', intent: 'press on', triggers: ['keep going'], + }); + }); + + it('POST transitions rejects a body with no target and never mints an id client-side', async () => { + const noTarget = await request(makeApp()) + .post('/api/fableloom/loom-1/episodes/ep-1/nodes/node-1/transitions') + .send({ intent: 'press on' }); + expect(noTarget.status).toBe(400); + expect(fableLoom.addNodeTransition).not.toHaveBeenCalled(); + + fableLoom.addNodeTransition.mockResolvedValueOnce({ loom: { id: 'loom-1' }, transition: { id: 'tr-2' } }); + await request(makeApp()) + .post('/api/fableloom/loom-1/episodes/ep-1/nodes/node-1/transitions') + .send({ targetNodeId: 'node-2', intent: 'press on', id: 'tr-mine' }); + expect(fableLoom.addNodeTransition).toHaveBeenCalledWith('loom-1', 'ep-1', 'node-1', { + targetNodeId: 'node-2', intent: 'press on', + }); + }); + + it('PATCH/DELETE transitions dispatch with the transition id', async () => { + fableLoom.updateNodeTransition.mockResolvedValueOnce({ id: 'loom-1' }); + const patched = await request(makeApp()) + .patch('/api/fableloom/loom-1/episodes/ep-1/nodes/node-1/transitions/tr-1') + .send({ intent: '' }); + expect(patched.status).toBe(200); + expect(fableLoom.updateNodeTransition).toHaveBeenCalledWith('loom-1', 'ep-1', 'node-1', 'tr-1', { intent: '' }); + + fableLoom.deleteNodeTransition.mockResolvedValueOnce({ id: 'loom-1' }); + const removed = await request(makeApp()) + .delete('/api/fableloom/loom-1/episodes/ep-1/nodes/node-1/transitions/tr-1'); + expect(removed.status).toBe(200); + expect(fableLoom.deleteNodeTransition).toHaveBeenCalledWith('loom-1', 'ep-1', 'node-1', 'tr-1'); + }); + + it('PATCH nodes still accepts the whole transitions array (back-compat)', async () => { + fableLoom.updateNode.mockResolvedValueOnce({ id: 'loom-1' }); + const rows = [{ id: 'tr-1', targetNodeId: 'node-2', intent: 'press on' }]; + const response = await request(makeApp()) + .patch('/api/fableloom/loom-1/episodes/ep-1/nodes/node-1') + .send({ transitions: rows }); + expect(response.status).toBe(200); + expect(fableLoom.updateNode).toHaveBeenCalledWith('loom-1', 'ep-1', 'node-1', { transitions: rows }); + }); + it('GET validate runs the deterministic analysis on the episode', async () => { fableLoom.getLoom.mockResolvedValueOnce({ id: 'loom-1', diff --git a/server/services/fableLoom/README.md b/server/services/fableLoom/README.md index 12ab9dfca..e6d1ad63d 100644 --- a/server/services/fableLoom/README.md +++ b/server/services/fableLoom/README.md @@ -7,7 +7,7 @@ intent to a transition and moves them through the graph until an ending. | Module | Purpose | |---|---| -| `records.js` | Sanitizer + CRUD for looms/episodes/nodes/transitions; `attachNodeImage` for the media-job hook. | +| `records.js` | Sanitizer + CRUD for looms/episodes/nodes; transitions are addressable one at a time (`addNodeTransition` / `updateNodeTransition` / `deleteNodeTransition`) as well as replaceable as a whole array via the node patch; `attachNodeImage` for the media-job hook. | | `weave.js` | AI ops via `runStagedLLM`: `weaveEpisode` (full graph), `branchNode` (grow paths), `reviewEpisode` (critique + deterministic analysis), `playTurn` (reader intent → transition; a tapped path resolves off the graph with NO LLM call), `reformatLoom` (rewrite every scene into another format). | | `formats.js` | Scene formats (`prose` / `teleplay`) and the prompt contracts each generative stage renders for them. | | `store.js` | PostgreSQL/file backend facade (`fableloom_stories`; collectionStore escape hatch for tests). | diff --git a/server/services/fableLoom/index.js b/server/services/fableLoom/index.js index 8d4b17537..3e99728a1 100644 --- a/server/services/fableLoom/index.js +++ b/server/services/fableLoom/index.js @@ -2,13 +2,16 @@ export { LOOM_LIMITS, addEpisode, addNode, + addNodeTransition, attachNodeImage, createLoom, deleteEpisode, deleteLoom, deleteNode, + deleteNodeTransition, findEpisode, findNode, + findTransition, getLoom, listLooms, listLoomSummaries, @@ -17,6 +20,7 @@ export { updateEpisode, updateLoom, updateNode, + updateNodeTransition, } from './records.js'; export { branchNode, diff --git a/server/services/fableLoom/records.js b/server/services/fableLoom/records.js index 52a9fd768..2cecd70a3 100644 --- a/server/services/fableLoom/records.js +++ b/server/services/fableLoom/records.js @@ -281,6 +281,12 @@ export const findNode = (episode, nodeId) => { return node; }; +export const findTransition = (node, transitionId) => { + const transition = (node.transitions || []).find((t) => t.id === transitionId); + if (!transition) throw notFound('Path'); + return transition; +}; + export function addEpisode(loomId, { title, synopsis } = {}) { return mutateLoom(loomId, (loom) => { if (loom.episodes.length >= LOOM_LIMITS.EPISODES_MAX) { @@ -374,6 +380,68 @@ export function deleteNode(loomId, episodeId, nodeId) { }); } +// --- Transitions as sub-resources ------------------------------------------- +// +// The whole-array `transitions` key on the node PATCH still works (the +// sanitizer owns it, and an older client or a peer on a previous version keeps +// using it). These three add/edit/drop ONE edge, so a second writer — a voice +// action, a CoS agent, the AI branch lane — no longer has to replay the whole +// array off a snapshot that may already be stale. + +const TRANSITION_PATCH_FIELDS = ['targetNodeId', 'intent', 'triggers', 'description']; + +// The sanitizer re-runs on every write, so the row that comes back out is the +// one that actually persisted (id, trimmed fields, capped triggers) — not the +// input echoed back. +const readTransition = (loom, episodeId, nodeId, transitionId) => + loom.episodes.find((e) => e.id === episodeId) + ?.nodes.find((n) => n.id === nodeId) + ?.transitions.find((t) => t.id === transitionId) ?? null; + +/** + * Add one path out of a scene. The id is minted here rather than inside the + * sanitizer so the caller can read the stored row back out by it — that is the + * whole point of the sub-resource: the client knows the id at create time and + * never has to reconcile server-minted ids into locally-added rows. + */ +export async function addNodeTransition(loomId, episodeId, nodeId, fields = {}) { + const id = `tr-${randomUUID()}`; + const loom = await mutateLoom(loomId, (record) => { + const episode = findEpisode(record, episodeId); + const node = findNode(episode, nodeId); + if ((node.transitions || []).length >= LOOM_LIMITS.TRANSITIONS_MAX) { + throw new ServerError('Path limit reached', { status: 400, code: 'LIMIT_REACHED' }); + } + node.transitions = [...(node.transitions || []), { ...fields, id }]; + episode.updatedAt = new Date().toISOString(); + return record; + }); + return { loom, transition: readTransition(loom, episodeId, nodeId, id) }; +} + +export function updateNodeTransition(loomId, episodeId, nodeId, transitionId, patch = {}) { + return mutateLoom(loomId, (record) => { + const episode = findEpisode(record, episodeId); + const transition = findTransition(findNode(episode, nodeId), transitionId); + for (const key of TRANSITION_PATCH_FIELDS) { + if (key in patch) transition[key] = patch[key]; + } + episode.updatedAt = new Date().toISOString(); + return record; + }); +} + +export function deleteNodeTransition(loomId, episodeId, nodeId, transitionId) { + return mutateLoom(loomId, (record) => { + const episode = findEpisode(record, episodeId); + const node = findNode(episode, nodeId); + findTransition(node, transitionId); + node.transitions = node.transitions.filter((t) => t.id !== transitionId); + episode.updatedAt = new Date().toISOString(); + return record; + }); +} + /** * Durable image attach for the media-job completion hook: files a finished * render onto its node, even when the editor unmounted mid-render. Returns the diff --git a/server/services/fableLoom/records.test.js b/server/services/fableLoom/records.test.js index 15671b630..17cc88150 100644 --- a/server/services/fableLoom/records.test.js +++ b/server/services/fableLoom/records.test.js @@ -24,9 +24,10 @@ const getSeriesMock = vi.hoisted(() => vi.fn(async (id) => ({ id }))); vi.mock('../pipeline/series.js', () => ({ getSeries: getSeriesMock })); const { - addEpisode, addNode, attachNodeImage, createLoom, deleteEpisode, deleteLoom, - deleteNode, getLoom, listLooms, listLoomSummaries, sanitizeLoom, updateEpisode, - updateLoom, updateNode, + LOOM_LIMITS, addEpisode, addNode, addNodeTransition, attachNodeImage, createLoom, + deleteEpisode, deleteLoom, deleteNode, deleteNodeTransition, getLoom, + listLooms, listLoomSummaries, sanitizeLoom, updateEpisode, updateLoom, + updateNode, updateNodeTransition, } = await import('./records.js'); const { _resetFableLoomBackend } = await import('./store.js'); @@ -256,6 +257,79 @@ describe('nodes and transitions', () => { }); }); +describe('transition sub-resources', () => { + const setup = async () => { + const created = await makeLoom(); + const withEp = await addEpisode(created.id, { title: 'Pilot' }); + const episodeId = withEp.episodes[0].id; + let updated = await addNode(created.id, episodeId, { title: 'A' }); + updated = await addNode(created.id, episodeId, { title: 'B' }); + updated = await addNode(created.id, episodeId, { title: 'C' }); + const [a, b, c] = updated.episodes[0].nodes; + return { loomId: created.id, episodeId, a, b, c }; + }; + const rowsOf = (record, episodeId, nodeId) => record.episodes.find((e) => e.id === episodeId) + .nodes.find((n) => n.id === nodeId).transitions; + + it('adds one path and hands back the minted row', async () => { + const { loomId, episodeId, a, b } = await setup(); + const { loom, transition } = await addNodeTransition(loomId, episodeId, a.id, { + targetNodeId: b.id, intent: 'press on', triggers: ['keep going', ''], + }); + expect(transition.id).toMatch(/^tr-/); + expect(transition).toMatchObject({ targetNodeId: b.id, intent: 'press on', triggers: ['keep going'] }); + expect(rowsOf(loom, episodeId, a.id)).toEqual([transition]); + }); + + it('adding a second path leaves the first one alone', async () => { + const { loomId, episodeId, a, b, c } = await setup(); + const first = (await addNodeTransition(loomId, episodeId, a.id, { targetNodeId: b.id, intent: 'left' })).transition; + const second = (await addNodeTransition(loomId, episodeId, a.id, { targetNodeId: c.id, intent: 'right' })).transition; + const rows = rowsOf(await getLoom(loomId), episodeId, a.id); + expect(rows.map((t) => t.id)).toEqual([first.id, second.id]); + expect(rows[0].intent).toBe('left'); + }); + + it('patches only the provided fields and keeps the id', async () => { + const { loomId, episodeId, a, b, c } = await setup(); + const { transition } = await addNodeTransition(loomId, episodeId, a.id, { + targetNodeId: b.id, intent: 'press on', triggers: ['keep going'], description: 'the long way', + }); + const updated = await updateNodeTransition(loomId, episodeId, a.id, transition.id, { + intent: '', targetNodeId: c.id, + }); + expect(rowsOf(updated, episodeId, a.id)[0]).toMatchObject({ + id: transition.id, + targetNodeId: c.id, + intent: '', + triggers: ['keep going'], + description: 'the long way', + }); + }); + + it('deletes one path without touching its siblings', async () => { + const { loomId, episodeId, a, b, c } = await setup(); + const doomed = (await addNodeTransition(loomId, episodeId, a.id, { targetNodeId: b.id, intent: 'left' })).transition; + const kept = (await addNodeTransition(loomId, episodeId, a.id, { targetNodeId: c.id, intent: 'right' })).transition; + const updated = await deleteNodeTransition(loomId, episodeId, a.id, doomed.id); + expect(rowsOf(updated, episodeId, a.id).map((t) => t.id)).toEqual([kept.id]); + }); + + it('404s on an unknown transition and refuses to exceed the cap', async () => { + const { loomId, episodeId, a, b } = await setup(); + await expect(updateNodeTransition(loomId, episodeId, a.id, 'tr-nope', { intent: 'x' })) + .rejects.toMatchObject({ status: 404 }); + await expect(deleteNodeTransition(loomId, episodeId, a.id, 'tr-nope')) + .rejects.toMatchObject({ status: 404 }); + + for (let i = 0; i < LOOM_LIMITS.TRANSITIONS_MAX; i += 1) { + await addNodeTransition(loomId, episodeId, a.id, { targetNodeId: b.id, intent: `path ${i}` }); + } + await expect(addNodeTransition(loomId, episodeId, a.id, { targetNodeId: b.id, intent: 'one too many' })) + .rejects.toMatchObject({ status: 400, code: 'LIMIT_REACHED' }); + }); +}); + describe('attachNodeImage', () => { it('files a completed render onto its node', async () => { const loom = await makeLoom();