diff --git a/.changeset/voice-provenance-rollback.md b/.changeset/voice-provenance-rollback.md new file mode 100644 index 00000000000..bd54895eba5 --- /dev/null +++ b/.changeset/voice-provenance-rollback.md @@ -0,0 +1,5 @@ +--- +"@hashintel/petrinaut": patch +--- + +Keep rejected overlapping Voice tool answers from restoring failed sibling provenance or overwriting newer message metadata. diff --git a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel.test.tsx b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel.test.tsx index a680b181f08..0fbd350333a 100644 --- a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel.test.tsx +++ b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel.test.tsx @@ -36,6 +36,7 @@ import { definePetrinautAiInteractiveTool } from "../../../types/ai-interactive- import { addMappedToolOutput, AiAssistantPanel, + getVoiceToolCallIds, safelyAddToolOutput, } from "./ai-assistant-panel"; @@ -157,6 +158,44 @@ const textChunks = (id: string, text: string): UIMessageChunk[] => [ { type: "text-end", id }, ]; +const createPendingVoiceQuestionMessage = ({ + id = "assistant-voice-questions", + metadata, + toolCallIds, +}: { + id?: string; + metadata?: PetrinautAiMessage["metadata"]; + toolCallIds: string[]; +}): PetrinautAiMessage => + ({ + id, + ...(metadata ? { metadata } : {}), + parts: toolCallIds.map((toolCallId) => ({ + input: { question: `Question for ${toolCallId}` }, + state: "input-available", + toolCallId, + toolName: "answerQuestion", + type: "dynamic-tool", + })), + role: "assistant", + }) as unknown as PetrinautAiMessage; + +const createTestMessageStore = (initialMessage: PetrinautAiMessage) => { + let messages = [initialMessage]; + + return { + getMessages: () => messages, + setMessages: (nextMessages: PetrinautAiMessage[]) => { + messages = nextMessages; + }, + updateMessages: ( + updater: (currentMessages: PetrinautAiMessage[]) => PetrinautAiMessage[], + ) => { + messages = updater(messages); + }, + }; +}; + const SubmitForSecondConversation = ({ conversationId, submitText, @@ -269,6 +308,21 @@ afterEach(() => { }); describe("AiAssistantPanel composer submissions", () => { + test("normalizes current and legacy voice tool origins", () => { + expect( + getVoiceToolCallIds({ + source: "voice", + toolCallId: "legacy-question", + voiceToolCallIds: [ + "current-question", + "legacy-question", + "current-question", + ], + }), + ).toEqual(["current-question", "legacy-question"]); + expect(getVoiceToolCallIds({ toolCallId: "legacy-question" })).toEqual([]); + }); + test("disables Clear when the host owns canonical conversation history", () => { const transport: PetrinautAiTransport = { reconnectToStream: () => Promise.resolve(null), @@ -3457,83 +3511,39 @@ describe("AiAssistantPanel composer submissions", () => { }); test("retains every voice tool origin on one assistant message", async () => { - let latestMessages = [ - { - id: "assistant-voice-questions", - parts: [ - { - input: { question: "Who approves it?" }, - state: "input-available", - toolCallId: "voice-question-1", - toolName: "answerQuestion", - type: "dynamic-tool", - }, - { - input: { question: "Who acts next?" }, - state: "input-available", - toolCallId: "voice-question-2", - toolName: "answerQuestion", - type: "dynamic-tool", - }, - ], - role: "assistant", - }, - ] as unknown as PetrinautAiMessage[]; - const updateMessages = ( - updater: (messages: PetrinautAiMessage[]) => PetrinautAiMessage[], - ) => { - latestMessages = updater(latestMessages); - }; + const messageStore = createTestMessageStore( + createPendingVoiceQuestionMessage({ + toolCallIds: ["voice-question-1", "voice-question-2"], + }), + ); const addToolOutput = vi.fn().mockResolvedValue(undefined); for (const toolCallId of ["voice-question-1", "voice-question-2"]) { await addMappedToolOutput({ addToolOutput, - currentMessages: latestMessages, + currentMessages: messageStore.getMessages(), params: { output: { answer: toolCallId }, tool: "answerQuestion", toolCallId, }, source: "voice", - updateMessages, + updateMessages: messageStore.updateMessages, }); } - expect(latestMessages[0]?.metadata).toEqual({ + expect(messageStore.getMessages()[0]?.metadata).toEqual({ source: "voice", voiceToolCallIds: ["voice-question-1", "voice-question-2"], }); }); test("preserves sibling voice provenance when another tool output rejects", async () => { - let latestMessages = [ - { - id: "assistant-voice-questions", - parts: [ - { - input: { question: "Who approves it?" }, - state: "input-available", - toolCallId: "voice-question-1", - toolName: "answerQuestion", - type: "dynamic-tool", - }, - { - input: { question: "Who acts next?" }, - state: "input-available", - toolCallId: "voice-question-2", - toolName: "answerQuestion", - type: "dynamic-tool", - }, - ], - role: "assistant", - }, - ] as unknown as PetrinautAiMessage[]; - const updateMessages = ( - updater: (messages: PetrinautAiMessage[]) => PetrinautAiMessage[], - ) => { - latestMessages = updater(latestMessages); - }; + const messageStore = createTestMessageStore( + createPendingVoiceQuestionMessage({ + toolCallIds: ["voice-question-1", "voice-question-2"], + }), + ); let rejectFirstSubmission: ((reason?: unknown) => void) | undefined; const addToolOutput = vi .fn() @@ -3547,14 +3557,14 @@ describe("AiAssistantPanel composer submissions", () => { const firstSubmission = addMappedToolOutput({ addToolOutput, - currentMessages: latestMessages, + currentMessages: messageStore.getMessages(), params: { output: { answer: "The shift lead" }, tool: "answerQuestion", toolCallId: "voice-question-1", }, source: "voice", - updateMessages, + updateMessages: messageStore.updateMessages, }); const firstSubmissionRejection = expect(firstSubmission).rejects.toThrow( "First voice tool output rejected.", @@ -3562,55 +3572,198 @@ describe("AiAssistantPanel composer submissions", () => { await addMappedToolOutput({ addToolOutput, - currentMessages: latestMessages, + currentMessages: messageStore.getMessages(), params: { output: { answer: "The release manager" }, tool: "answerQuestion", toolCallId: "voice-question-2", }, source: "voice", - updateMessages, + updateMessages: messageStore.updateMessages, }); rejectFirstSubmission?.(new Error("First voice tool output rejected.")); await firstSubmissionRejection; - expect(latestMessages[0]?.metadata).toEqual({ + expect(messageStore.getMessages()[0]?.metadata).toEqual({ source: "voice", voiceToolCallIds: ["voice-question-2"], }); }); - test("rolls back failed tool provenance before a typed retry", async () => { - let latestMessages = [ - { - id: "assistant-pending-voice-question", - parts: [ - { - input: { question: "Who approves it?" }, - state: "input-available", - toolCallId: "voice-question", - toolName: "answerQuestion", - type: "dynamic-tool", - }, - ], - role: "assistant", - }, - ] as unknown as PetrinautAiMessage[]; - const updateMessages = ( - updater: (messages: PetrinautAiMessage[]) => PetrinautAiMessage[], - ) => { - latestMessages = updater(latestMessages); - }; - const addToolOutput = vi - .fn() - .mockImplementationOnce(async () => { - latestMessages = latestMessages.map((message) => ({ + test.each(["first-then-second", "second-then-first"] as const)( + "removes both failed voice origins when overlapping outputs reject %s", + async (rejectionOrder) => { + const messageStore = createTestMessageStore( + createPendingVoiceQuestionMessage({ + toolCallIds: ["voice-question-1", "voice-question-2"], + }), + ); + let rejectFirstSubmission: ((reason?: unknown) => void) | undefined; + let rejectSecondSubmission: ((reason?: unknown) => void) | undefined; + const addToolOutput = vi + .fn() + .mockImplementationOnce( + () => + new Promise((_resolve, reject) => { + rejectFirstSubmission = reject; + }), + ) + .mockImplementationOnce( + () => + new Promise((_resolve, reject) => { + rejectSecondSubmission = reject; + }), + ); + + const firstSubmission = addMappedToolOutput({ + addToolOutput, + currentMessages: messageStore.getMessages(), + params: { + output: { answer: "The shift lead" }, + tool: "answerQuestion", + toolCallId: "voice-question-1", + }, + source: "voice", + updateMessages: messageStore.updateMessages, + }); + const firstSubmissionRejection = expect(firstSubmission).rejects.toThrow( + "First voice tool output rejected.", + ); + const secondSubmission = addMappedToolOutput({ + addToolOutput, + currentMessages: messageStore.getMessages(), + params: { + output: { answer: "The release manager" }, + tool: "answerQuestion", + toolCallId: "voice-question-2", + }, + source: "voice", + updateMessages: messageStore.updateMessages, + }); + const secondSubmissionRejection = expect( + secondSubmission, + ).rejects.toThrow("Second voice tool output rejected."); + + if (rejectionOrder === "first-then-second") { + rejectFirstSubmission?.(new Error("First voice tool output rejected.")); + await firstSubmissionRejection; + rejectSecondSubmission?.( + new Error("Second voice tool output rejected."), + ); + await secondSubmissionRejection; + } else { + rejectSecondSubmission?.( + new Error("Second voice tool output rejected."), + ); + await secondSubmissionRejection; + rejectFirstSubmission?.(new Error("First voice tool output rejected.")); + await firstSubmissionRejection; + } + + expect(messageStore.getMessages()[0]?.metadata).toBeUndefined(); + }, + ); + + test("preserves independent voice provenance and concurrent message updates on rejection", async () => { + const messageStore = createTestMessageStore( + createPendingVoiceQuestionMessage({ + id: "assistant-voice-question", + metadata: { source: "voice" }, + toolCallIds: ["voice-question"], + }), + ); + const addToolOutput = vi.fn().mockImplementationOnce(async () => { + messageStore.setMessages( + messageStore.getMessages().map((message) => ({ ...message, + metadata: { ...message.metadata, stopped: true }, parts: [ ...message.parts, { text: "Unrelated concurrent update", type: "text" }, ], - })) as PetrinautAiMessage[]; + })) as PetrinautAiMessage[], + ); + throw new Error("Voice tool output rejected."); + }); + + await expect( + addMappedToolOutput({ + addToolOutput, + currentMessages: messageStore.getMessages(), + params: { + output: { answer: "The shift lead" }, + tool: "answerQuestion", + toolCallId: "voice-question", + }, + source: "voice", + updateMessages: messageStore.updateMessages, + }), + ).rejects.toThrow("Voice tool output rejected."); + + expect(messageStore.getMessages()[0]?.metadata).toEqual({ + source: "voice", + stopped: true, + }); + expect(messageStore.getMessages()[0]?.parts).toContainEqual({ + text: "Unrelated concurrent update", + type: "text", + }); + }); + + test("preserves pre-existing voice attribution for a rejected tool", async () => { + const messageStore = createTestMessageStore( + createPendingVoiceQuestionMessage({ + id: "assistant-voice-question", + metadata: { + source: "voice", + voiceToolCallIds: ["voice-question"], + }, + toolCallIds: ["voice-question"], + }), + ); + const addToolOutput = vi + .fn() + .mockRejectedValueOnce(new Error("Voice tool output rejected.")); + + await expect( + addMappedToolOutput({ + addToolOutput, + currentMessages: messageStore.getMessages(), + params: { + output: { answer: "The shift lead" }, + tool: "answerQuestion", + toolCallId: "voice-question", + }, + source: "voice", + updateMessages: messageStore.updateMessages, + }), + ).rejects.toThrow("Voice tool output rejected."); + + expect(messageStore.getMessages()[0]?.metadata).toEqual({ + source: "voice", + voiceToolCallIds: ["voice-question"], + }); + }); + + test("rolls back failed tool provenance before a typed retry", async () => { + const messageStore = createTestMessageStore( + createPendingVoiceQuestionMessage({ + id: "assistant-pending-voice-question", + toolCallIds: ["voice-question"], + }), + ); + const addToolOutput = vi + .fn() + .mockImplementationOnce(async () => { + messageStore.setMessages( + messageStore.getMessages().map((message) => ({ + ...message, + parts: [ + ...message.parts, + { text: "Unrelated concurrent update", type: "text" }, + ], + })) as PetrinautAiMessage[], + ); throw new Error("Voice tool output rejected."); }) .mockResolvedValueOnce(undefined); @@ -3623,27 +3776,27 @@ describe("AiAssistantPanel composer submissions", () => { await expect( addMappedToolOutput({ addToolOutput, - currentMessages: latestMessages, + currentMessages: messageStore.getMessages(), params, source: "voice", - updateMessages, + updateMessages: messageStore.updateMessages, }), ).rejects.toThrow("Voice tool output rejected."); - expect(latestMessages[0]?.metadata).toBeUndefined(); - expect(latestMessages[0]?.parts).toContainEqual({ + expect(messageStore.getMessages()[0]?.metadata).toBeUndefined(); + expect(messageStore.getMessages()[0]?.parts).toContainEqual({ text: "Unrelated concurrent update", type: "text", }); await addMappedToolOutput({ addToolOutput, - currentMessages: latestMessages, + currentMessages: messageStore.getMessages(), params: { ...params, output: { answer: "Typed retry" }, }, - updateMessages, + updateMessages: messageStore.updateMessages, }); expect(addToolOutput).toHaveBeenLastCalledWith({ @@ -3651,7 +3804,7 @@ describe("AiAssistantPanel composer submissions", () => { tool: "answerQuestion", toolCallId: "voice-question", }); - expect(latestMessages[0]?.metadata).toBeUndefined(); + expect(messageStore.getMessages()[0]?.metadata).toBeUndefined(); }); test("reports browser tool-output rejections through the AI SDK error state", async () => { diff --git a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel.tsx b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel.tsx index 0979fd72005..e931eb94981 100644 --- a/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel.tsx +++ b/libs/@hashintel/petrinaut/src/ui/views/Editor/panels/ai-assistant-panel.tsx @@ -160,6 +160,18 @@ const voiceInputWithdrawn = (signal: AbortSignal | undefined): unknown => "AbortError", ); +export const getVoiceToolCallIds = ( + metadata: PetrinautAiMessage["metadata"], +): string[] => + metadata?.source === "voice" + ? [ + ...new Set([ + ...(metadata.voiceToolCallIds ?? []), + ...(metadata.toolCallId ? [metadata.toolCallId] : []), + ]), + ] + : []; + const markVoiceToolOrigin = ( messages: PetrinautAiMessage[], messageId: string, @@ -168,15 +180,7 @@ const markVoiceToolOrigin = ( messages.map((message) => message.id === messageId ? (() => { - const previousToolCallIds = - message.metadata?.source === "voice" - ? [ - ...(message.metadata.voiceToolCallIds ?? []), - ...(message.metadata.toolCallId - ? [message.metadata.toolCallId] - : []), - ] - : []; + const previousToolCallIds = getVoiceToolCallIds(message.metadata); const { toolCallId: _legacyToolCallId, ...previousMetadata } = message.metadata ?? {}; @@ -256,6 +260,74 @@ const addDynamicToolOutput = ( return Promise.resolve(addToolOutputForDynamicTool(params)); }; +type UpdatePetrinautAiMessages = ( + updater: (messages: PetrinautAiMessage[]) => PetrinautAiMessage[], +) => void; + +type VoiceToolSubmissionState = { + pendingSubmissionCount: number; + preexistingSource: boolean; + preexistingToolCallIds: Set; +}; + +const voiceToolSubmissionStates = new WeakMap< + UpdatePetrinautAiMessages, + Map +>(); + +const beginVoiceToolSubmission = ( + updateMessages: UpdatePetrinautAiMessages, + message: PetrinautAiMessage, +): VoiceToolSubmissionState => { + let messageStates = voiceToolSubmissionStates.get(updateMessages); + if (!messageStates) { + messageStates = new Map(); + voiceToolSubmissionStates.set(updateMessages, messageStates); + } + + let submissionState = messageStates.get(message.id); + if (!submissionState) { + submissionState = { + pendingSubmissionCount: 0, + preexistingSource: message.metadata?.source === "voice", + preexistingToolCallIds: new Set(getVoiceToolCallIds(message.metadata)), + }; + messageStates.set(message.id, submissionState); + } + + submissionState.pendingSubmissionCount += 1; + return submissionState; +}; + +const finishVoiceToolSubmission = ( + updateMessages: UpdatePetrinautAiMessages, + messageId: string, +): void => { + const messageStates = voiceToolSubmissionStates.get(updateMessages); + if (!messageStates) { + return; + } + + const submissionState = messageStates.get(messageId); + if (!submissionState) { + return; + } + + const pendingSubmissionCount = submissionState.pendingSubmissionCount - 1; + if (pendingSubmissionCount > 0) { + messageStates.set(messageId, { + ...submissionState, + pendingSubmissionCount, + }); + return; + } + + messageStates.delete(messageId); + if (messageStates.size === 0) { + voiceToolSubmissionStates.delete(updateMessages); + } +}; + export const addMappedToolOutput = async ({ addToolOutput, currentMessages, @@ -283,7 +355,9 @@ export const addMappedToolOutput = async ({ ), ) : undefined; - const previousMetadata = containingMessage?.metadata; + const submissionState = containingMessage + ? beginVoiceToolSubmission(updateMessages, containingMessage) + : undefined; if (containingMessage) { updateMessages((latestMessages) => @@ -300,50 +374,61 @@ export const addMappedToolOutput = async ({ } catch (error) { if (containingMessage) { updateMessages((latestMessages) => - latestMessages.map((message) => - message.id === containingMessage.id && - message.metadata?.source === "voice" && - (message.metadata.voiceToolCallIds?.includes(params.toolCallId) === - true || - message.metadata.toolCallId === params.toolCallId) - ? (() => { - const attributionAlreadyPresent = - previousMetadata?.source === "voice" && - (previousMetadata.voiceToolCallIds?.includes( - params.toolCallId, - ) === true || - previousMetadata.toolCallId === params.toolCallId); - const voiceToolCallIds = [ - ...(message.metadata.voiceToolCallIds ?? []), - ...(message.metadata.toolCallId - ? [message.metadata.toolCallId] - : []), - ]; - const remainingVoiceToolCallIds = attributionAlreadyPresent - ? voiceToolCallIds - : voiceToolCallIds.filter( - (candidateToolCallId) => - candidateToolCallId !== params.toolCallId, - ); - if (remainingVoiceToolCallIds.length === 0) { - return { ...message, metadata: previousMetadata }; - } - const { toolCallId: _legacyToolCallId, ...metadata } = - message.metadata; - - return { - ...message, - metadata: { - ...metadata, - voiceToolCallIds: [...new Set(remainingVoiceToolCallIds)], - }, - }; - })() - : message, - ), + latestMessages.map((message) => { + if ( + message.id !== containingMessage.id || + message.metadata?.source !== "voice" + ) { + return message; + } + + const voiceToolCallIds = getVoiceToolCallIds(message.metadata); + if (!voiceToolCallIds.includes(params.toolCallId)) { + return message; + } + + const attributionAlreadyPresent = + submissionState?.preexistingToolCallIds.has(params.toolCallId) === + true; + const remainingVoiceToolCallIds = attributionAlreadyPresent + ? voiceToolCallIds + : voiceToolCallIds.filter( + (candidateToolCallId) => + candidateToolCallId !== params.toolCallId, + ); + if (remainingVoiceToolCallIds.length === 0) { + const { + source: _source, + toolCallId: _legacyToolCallId, + voiceToolCallIds: _voiceToolCallIds, + ...unrelatedMetadata + } = message.metadata; + const metadata = submissionState?.preexistingSource + ? { ...unrelatedMetadata, source: "voice" as const } + : Object.keys(unrelatedMetadata).length > 0 + ? unrelatedMetadata + : undefined; + + return { ...message, metadata }; + } + const { toolCallId: _legacyToolCallId, ...metadata } = + message.metadata; + + return { + ...message, + metadata: { + ...metadata, + voiceToolCallIds: remainingVoiceToolCallIds, + }, + }; + }), ); } throw error; + } finally { + if (containingMessage && submissionState) { + finishVoiceToolSubmission(updateMessages, containingMessage.id); + } } };