From 90f3a622e9a66faf6cba1d80dafc7936368277b8 Mon Sep 17 00:00:00 2001 From: Aari Date: Thu, 23 Jul 2026 21:58:55 +0800 Subject: [PATCH] fix(frontend): keep leading orphan tool messages visible (#4408) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(frontend): keep leading orphan tool messages visible #3880 stopped dropping orphan tool messages that arrive after a terminal group, but left the leading-orphan branch dropping the message with a console.error on every render. The case is reachable: history pagination cuts by event seq, not turn boundaries, so the first loaded page can begin mid-turn with tool results whose AI tool-call message sits on an unloaded older page — in dev mode the diagnostic surfaces as a full-screen Next.js error overlay (#4399). Open a processing group for the leading orphan instead; loading the older page re-groups it under its real turn. * test(frontend): lock leading-orphan accumulation invariant; clarify self-heal comment Address review on #4408: - The self-heal note only holds for the pagination path. The hidden-only-precedent case and a truly orphaned tool have no older page to load, so the processing group persists and renders as an empty ChainOfThought shell (convertToSteps emits steps only for type === ai). Document that as an accepted degradation instead of implying it always self-heals. - Add a test locking the accumulation invariant: a leading orphan followed by a real tool-call turn folds into one processing group, not a stranded empty group beside the real turn. --- frontend/src/core/messages/utils.ts | 28 ++++-- .../tests/unit/core/messages/utils.test.ts | 99 +++++++++++++++++++ 2 files changed, 120 insertions(+), 7 deletions(-) diff --git a/frontend/src/core/messages/utils.ts b/frontend/src/core/messages/utils.ts index 131290803..6c72d5478 100644 --- a/frontend/src/core/messages/utils.ts +++ b/frontend/src/core/messages/utils.ts @@ -92,13 +92,27 @@ export function getMessageGroups(messages: Message[]): MessageGroup[] { if (lastGroup) { lastGroup.messages.push(message); } else { - // groups is empty (shouldn't happen — the outer for loop is guarded - // by `messages.length === 0 -> return []`), but keep the diagnostic - // just in case. - console.error( - "Unexpected tool message with no preceding group", - message, - ); + // Leading orphan: `groups` is empty when this tool message + // arrives. Two paths reach here: (1) history pagination cuts by + // event seq, not turn boundaries, so the first loaded page begins + // mid-turn with a tool result whose AI tool-call sits on an + // unloaded older page (#4399); (2) the tool message is preceded + // only by hidden control messages. Open a processing group so it + // stays visible instead of being dropped with a per-render console + // error. + // + // Only case (1) self-heals — loading the older page re-groups the + // tool under its real turn. Case (2), and any truly orphaned tool + // with no AI antecedent, has no page to load: the group persists + // and renders as an empty ChainOfThought shell (convertToSteps + // emits steps only for `type === "ai"`). That empty shell is an + // accepted degradation — still a net win over dropping the result + // and firing console.error every render. + groups.push({ + id: message.id, + type: "assistant:processing", + messages: [message], + }); } } } diff --git a/frontend/tests/unit/core/messages/utils.test.ts b/frontend/tests/unit/core/messages/utils.test.ts index 750abe39e..69a718cd0 100644 --- a/frontend/tests/unit/core/messages/utils.test.ts +++ b/frontend/tests/unit/core/messages/utils.test.ts @@ -724,6 +724,105 @@ describe("orphan tool messages", () => { expect(t2?.type).toBe("tool"); }); + test("opens a processing group for a leading orphan tool message", () => { + // History pagination cuts by event seq, not turn boundaries, so the first + // loaded page can begin mid-turn with tool results whose AI tool-call + // message sits on an unloaded older page (#4399). They must stay visible + // in a processing group, not be dropped with a console error per render. + const messages = [ + { + id: "t-lead-1", + type: "tool", + name: "bash", + tool_call_id: "call-old-1", + content: "output from unloaded turn", + }, + { + id: "t-lead-2", + type: "tool", + name: "bash", + tool_call_id: "call-old-2", + content: "second output", + }, + { id: "ai-1", type: "ai", content: "Done." }, + ] as Message[]; + + const groups = getMessageGroups(messages); + + expect(groups.map((g) => g.type)).toEqual([ + "assistant:processing", + "assistant", + ]); + // Both leading orphans cluster into the same processing group. + expect(groups[0]?.messages.map((m) => m.id)).toEqual([ + "t-lead-1", + "t-lead-2", + ]); + }); + + test("leading orphan absorbs the following real AI turn into one processing group", () => { + // A leading orphan followed by a real tool-call turn must accumulate into + // a single processing group via the assistant:processing branch, not + // strand the orphan as a separate empty group beside the real turn. + const messages = [ + { + id: "t-lead", + type: "tool", + name: "bash", + tool_call_id: "call-old", + content: "output from unloaded turn", + }, + { + id: "ai-1", + type: "ai", + content: "running", + tool_calls: [{ id: "call-1", name: "bash", args: {} }], + }, + { + id: "t-1", + type: "tool", + name: "bash", + tool_call_id: "call-1", + content: "output-1", + }, + ] as Message[]; + + const groups = getMessageGroups(messages); + + // One processing group holds the orphan, the AI turn, and its tool result. + expect(groups.map((g) => g.type)).toEqual(["assistant:processing"]); + expect(groups[0]?.messages.map((m) => m.id)).toEqual([ + "t-lead", + "ai-1", + "t-1", + ]); + }); + + test("tool message preceded only by hidden messages gets a group", () => { + // messages is non-empty, but every earlier message is hidden from the UI — + // groups is still empty when the tool message arrives. + const messages = [ + { + id: "hidden-1", + type: "human", + content: + "\n# SKILL.md\n", + }, + { + id: "t-1", + type: "tool", + name: "bash", + tool_call_id: "call-1", + content: "output", + }, + ] as Message[]; + + const groups = getMessageGroups(messages); + + expect(groups.map((g) => g.type)).toEqual(["assistant:processing"]); + expect(groups[0]?.messages.map((m) => m.id)).toEqual(["t-1"]); + }); + test("replayed tool with same tool_call_id is not lost (duplicate stream events)", () => { // LangGraph subagent state restoration can replay tool-result events. The // frontend log shows the same tool_call_id arriving twice. Both occurrences