Просмотр исходного кода

fix(app): preserve assistant content order (#42721)

Luke Parker 2 дней назад
Родитель
Сommit
a5f3e9e735

+ 1 - 9
packages/app/e2e/performance/timeline/session-timeline-stress.fixture.ts

@@ -275,10 +275,6 @@ function renderable(part: MessagePart) {
   return part.type !== "step-start" && part.type !== "step-finish" && part.type !== "patch"
 }
 
-function orderedParts(message: Message) {
-  return message.parts.slice().sort((a, b) => a.id.localeCompare(b.id))
-}
-
 export const fixture = {
   directory,
   project: {
@@ -345,11 +341,7 @@ export const fixture = {
       .filter((message) => message.info.role === "user")
       .map((message) => message.info.id),
     childMessageIDs: childMessages.filter((message) => message.info.role === "user").map((message) => message.info.id),
-    targetPartIDs: targetMessages.flatMap((message) =>
-      orderedParts(message)
-        .filter(renderable)
-        .map((part) => part.id),
-    ),
+    targetPartIDs: targetMessages.flatMap((message) => message.parts.filter(renderable).map((part) => part.id)),
   },
 }
 

+ 3 - 4
packages/app/e2e/regression/session-timeline-reducer-projection.spec.ts

@@ -23,11 +23,10 @@ test("groups singleton and separated context operations at correct boundaries",
   ]
   await setupTimeline(page, { messages: [userMessage(), assistantMessage(parts)] })
 
-  await expect(
-    page.locator('[data-timeline-part-ids="prt_boundary_01_read,prt_boundary_03_glob,prt_boundary_04_grep"]'),
-  ).toBeVisible()
+  await expect(page.locator('[data-timeline-part-ids="prt_boundary_01_read"]')).toBeVisible()
+  await expect(page.locator('[data-timeline-part-ids="prt_boundary_03_glob,prt_boundary_04_grep"]')).toBeVisible()
   await expect(page.locator('[data-timeline-part-ids="prt_boundary_06_list"]')).toBeVisible()
-  await expect(page.locator('[data-timeline-row="AssistantPart"]')).toHaveCount(4)
+  await expect(page.locator('[data-timeline-row="AssistantPart"]')).toHaveCount(5)
 })
 
 test("reducer-hardening: converges when idle arrives before final part and message completion", async ({ page }) => {

+ 0 - 1
packages/app/e2e/smoke/session-timeline.fixture.ts

@@ -245,7 +245,6 @@ function currentPartIDs(message: Message) {
       if (part.type === "tool") return [typeof part.callID === "string" ? part.callID : part.id]
       return []
     })
-    .sort()
 }
 
 export const fixture = {

+ 69 - 6
packages/app/src/context/server-session.test.ts

@@ -370,14 +370,38 @@ describe("server session", () => {
       location: { directory: "/repo" },
       data: { sessionID: "child", assistantMessageID: "msg_2_assistant", ordinal: 0, delta: "world" },
     })
+    apply({
+      id: "evt_tool_z",
+      created: 5,
+      type: "session.tool.input.started",
+      durable: { aggregateID: "child", seq: 3, version: 1 },
+      location: { directory: "/repo" },
+      data: { sessionID: "child", assistantMessageID: "msg_2_assistant", id: "call_z", name: "shell" },
+    })
+    apply({
+      id: "evt_tool_a",
+      created: 6,
+      type: "session.tool.input.started",
+      durable: { aggregateID: "child", seq: 4, version: 1 },
+      location: { directory: "/repo" },
+      data: { sessionID: "child", assistantMessageID: "msg_2_assistant", id: "call_a", name: "shell" },
+    })
 
     expect(ctx.store.data.session_message.child?.at(-1)).toMatchObject({
       id: "msg_2_assistant",
       type: "assistant",
-      content: [{ type: "text", text: "world" }],
+      content: [
+        { type: "text", text: "world" },
+        { type: "tool", id: "call_z" },
+        { type: "tool", id: "call_a" },
+      ],
     })
     expect(ctx.store.data.message.child?.map((message) => message.id)).toEqual(["msg_1_user", "msg_2_assistant"])
-    expect(ctx.store.data.part.msg_2_assistant).toMatchObject([{ type: "text", text: "world" }])
+    expect(ctx.store.data.part.msg_2_assistant?.map((part) => part.id)).toEqual([
+      "msg_2_assistant:text:0",
+      "call_z",
+      "call_a",
+    ])
   })
 
   test("projects V2 pending inputs and forms", () => {
@@ -636,6 +660,45 @@ describe("server session", () => {
     expect(store.data.message.root.map((message) => message.id)).toEqual([user.id, assistant.id])
   })
 
+  test("preserves assistant content order from message history", async () => {
+    const source = [
+      { id: "msg_user", type: "user", text: "inspect it", time: { created: 1 } },
+      {
+        id: "msg_assistant",
+        type: "assistant",
+        agent: "build",
+        model: { id: "model", providerID: "provider" },
+        content: [
+          { type: "text", text: "I will inspect it." },
+          {
+            type: "tool",
+            id: "call_z",
+            name: "shell",
+            state: { status: "streaming", input: "" },
+            time: { created: 2 },
+          },
+          {
+            type: "tool",
+            id: "call_a",
+            name: "shell",
+            state: { status: "streaming", input: "" },
+            time: { created: 3 },
+          },
+        ],
+        time: { created: 2 },
+      },
+    ] satisfies SessionMessageInfo[]
+    const messageApi = {
+      list: async () => ({ data: source.toReversed(), cursor: { previous: null, next: null } }),
+    } as unknown as MessageApi
+    const store = createServerSession({} as SessionApi, messageApi)
+    store.remember(session("root"))
+
+    await store.sync("root")
+
+    expect(store.data.part.msg_assistant?.map((part) => part.id)).toEqual(["msg_assistant:text:0", "call_z", "call_a"])
+  })
+
   test("extends a current page to include the user for split assistant turns", async () => {
     const user = { id: "msg_1_user", type: "user", text: "hello", time: { created: 1 } } as const
     const assistant = (id: string, created: number) => ({
@@ -1091,6 +1154,8 @@ describe("server session", () => {
     expect(store.data.session_message.child).toBeUndefined()
     expect(store.data.message.child?.map((message) => message.id)).toEqual(["msg_prompt"])
     expect(store.data.part.msg_prompt).toMatchObject([
+      { id: "msg_prompt:text:0", type: "text", text: "hello" },
+      { id: "msg_prompt:file:0", type: "file", filename: "foo.ts" },
       { id: "msg_prompt:agent:0", type: "agent", name: "explore" },
       {
         id: "msg_prompt:comment:0",
@@ -1106,8 +1171,6 @@ describe("server session", () => {
           },
         },
       },
-      { id: "msg_prompt:file:0", type: "file", filename: "foo.ts" },
-      { id: "msg_prompt:text:0", type: "text", text: "hello" },
     ])
 
     store.applyV2({
@@ -1129,8 +1192,8 @@ describe("server session", () => {
     } as OpenCodeEvent)
 
     expect(store.data.part.msg_prompt).toMatchObject([
-      { id: "msg_prompt:comment:0", type: "text", synthetic: true },
       { id: "msg_prompt:text:0", type: "text", text: "hello" },
+      { id: "msg_prompt:comment:0", type: "text", synthetic: true },
     ])
   })
 
@@ -1174,8 +1237,8 @@ describe("server session", () => {
     await store.sync("child")
 
     expect(store.data.part.msg_prompt).toMatchObject([
-      { id: "msg_prompt:comment:0", type: "text", synthetic: true },
       { id: "msg_prompt:text:0", type: "text", text: "hello" },
+      { id: "msg_prompt:comment:0", type: "text", synthetic: true },
     ])
   })
 

+ 15 - 23
packages/app/src/context/server-session.ts

@@ -161,6 +161,7 @@ export function createServerSession(
     input: {} as Record<string, string[]>,
     message: {} as Record<string, Message[]>,
     session_message: {} as Record<string, SessionMessageInfo[]>,
+    // Part order is semantic and follows SessionMessageAssistant.content; IDs identify parts only.
     part: {} as Record<string, Part[]>,
     part_text_accum_delta: {} as Record<string, string>,
     session_working(id: string) {
@@ -217,7 +218,7 @@ export function createServerSession(
       if (part.id !== `${messageID}:text:0` || part.type !== "text") return [part]
       return text?.type === "text" && text.text ? [{ ...part, text: text.text }] : []
     })
-    return merge(projected, comments)
+    return [...projected, ...comments]
   }
   const deleteMessageParts = (
     cache: { part: Record<string, Part[] | undefined>; part_text_accum_delta: Record<string, string | undefined> },
@@ -474,9 +475,7 @@ export function createServerSession(
     const normalized = normalizeSessionMessages(sessionID, source)
     return {
       session: normalized.messages.sort(compareMessages),
-      part: [...normalized.parts.entries()]
-        .map(([id, part]) => ({ id, part: part.sort((a, b) => cmp(a.id, b.id)) }))
-        .sort((a, b) => cmp(a.id, b.id)),
+      part: [...normalized.parts.entries()].map(([id, part]) => ({ id, part })).sort((a, b) => cmp(a.id, b.id)),
       source,
       sourceMode: before ? ("older" as const) : ("latest" as const),
       projectSource: true,
@@ -607,9 +606,7 @@ export function createServerSession(
             return {
               ...page,
               session: normalized.messages.sort(compareMessages),
-              part: [...normalized.parts.entries()]
-                .map(([id, part]) => ({ id, part: part.sort((a, b) => cmp(a.id, b.id)) }))
-                .sort((a, b) => cmp(a.id, b.id)),
+              part: [...normalized.parts.entries()].map(([id, part]) => ({ id, part })).sort((a, b) => cmp(a.id, b.id)),
             }
           })()
         : page
@@ -823,7 +820,7 @@ export function createServerSession(
         for (const part of next) {
           apply({ type: "message.part.updated", properties: { sessionID: reduction.sessionID, part } })
         }
-        for (const part of data.part[messageID] ?? []) {
+        for (const part of [...(data.part[messageID] ?? [])]) {
           if (nextIDs.has(part.id)) continue
           apply({
             type: "message.part.removed",
@@ -1191,14 +1188,9 @@ export function createServerSession(
           setData("part", part.messageID, [part])
           return
         }
-        const result = Binary.search(parts, part.id, (item) => item.id)
-        if (result.found) setData("part", part.messageID, result.index, reconcile(part))
-        if (!result.found)
-          setData("part", part.messageID, (value = []) => {
-            const next = value.slice()
-            next.splice(result.index, 0, part)
-            return next
-          })
+        const index = parts.findIndex((item) => item.id === part.id)
+        if (index >= 0) setData("part", part.messageID, index, reconcile(part))
+        if (index < 0) setData("part", part.messageID, (value = []) => [...value, part])
         return
       }
       case "message.part.removed": {
@@ -1228,8 +1220,8 @@ export function createServerSession(
             deltaBases.delete(props.partID)
             const parts = draft.part[props.messageID]
             if (!parts) return
-            const result = Binary.search(parts, props.partID, (part) => part.id)
-            if (result.found) parts.splice(result.index, 1)
+            const index = parts.findIndex((part) => part.id === props.partID)
+            if (index >= 0) parts.splice(index, 1)
             if (parts.length === 0) delete draft.part[props.messageID]
           }),
         )
@@ -1245,8 +1237,8 @@ export function createServerSession(
         }
         const parts = data.part[props.messageID]
         if (!parts) return
-        const result = Binary.search(parts, props.partID, (part) => part.id)
-        if (!result.found) return
+        const index = parts.findIndex((part) => part.id === props.partID)
+        if (index < 0) return
         trackPartChange(props.sessionID, props.messageID, props.partID)
         const load = messageLoads.get(props.sessionID)
         if (load) {
@@ -1258,7 +1250,7 @@ export function createServerSession(
           if (carried?.size === 0) load.carriedDeltaParts.delete(props.messageID)
         }
         const field = props.field as keyof (typeof parts)[number]
-        const current = parts[result.index]?.[field]
+        const current = parts[index]?.[field]
         if (!deltaBases.has(props.partID) && typeof current === "string")
           deltaBases.set(props.partID, { base: current, sessionID: props.sessionID })
         setData(
@@ -1271,7 +1263,7 @@ export function createServerSession(
           props.messageID,
           produce((draft) => {
             if (!draft) return
-            const part = draft[result.index]
+            const part = draft[index]
             const field = props.field as keyof typeof part
             ;(part[field] as string) = ((part[field] as string | undefined) ?? "") + props.delta
           }),
@@ -1420,7 +1412,7 @@ export function createServerSession(
           synthetic: true,
           metadata: createCommentMetadata(comment),
         }))
-        const parts = merge(projected.parts.get(input.messageID) ?? [], comments).sort((a, b) => cmp(a.id, b.id))
+        const parts = [...(projected.parts.get(input.messageID) ?? []), ...comments]
         removedMessages.get(input.sessionID)?.delete(input.messageID)
         markEcho(input.sessionID, input.messageID)
         pendingRevision.set(input.sessionID, (pendingRevision.get(input.sessionID) ?? 0) + 1)