Apply PR #14307: fix: use parentID matching instead of ID ordering for prompt loop exit and message rendering
commit
07e958b843
|
|
@ -0,0 +1,81 @@
|
||||||
|
import { describe, expect, test } from "bun:test"
|
||||||
|
import type { Message } from "@opencode-ai/sdk/v2/client"
|
||||||
|
import { findAssistantMessages } from "@opencode-ai/ui/find-assistant-messages"
|
||||||
|
|
||||||
|
function user(id: string): Message {
|
||||||
|
return {
|
||||||
|
id,
|
||||||
|
role: "user",
|
||||||
|
sessionID: "session-1",
|
||||||
|
time: { created: 1 },
|
||||||
|
} as unknown as Message
|
||||||
|
}
|
||||||
|
|
||||||
|
function assistant(id: string, parentID: string): Message {
|
||||||
|
return {
|
||||||
|
id,
|
||||||
|
role: "assistant",
|
||||||
|
sessionID: "session-1",
|
||||||
|
parentID,
|
||||||
|
time: { created: 1 },
|
||||||
|
} as unknown as Message
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("findAssistantMessages", () => {
|
||||||
|
test("normal ordering: assistant after user in array → found via forward scan", () => {
|
||||||
|
const messages = [user("u1"), assistant("a1", "u1")]
|
||||||
|
const result = findAssistantMessages(messages, 0, "u1")
|
||||||
|
expect(result).toHaveLength(1)
|
||||||
|
expect(result[0].id).toBe("a1")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("clock skew: assistant before user in array → found via backward scan", () => {
|
||||||
|
// When client clock is ahead, user ID sorts after assistant ID,
|
||||||
|
// so assistant appears earlier in the ID-sorted message array
|
||||||
|
const messages = [assistant("a1", "u1"), user("u1")]
|
||||||
|
const result = findAssistantMessages(messages, 1, "u1")
|
||||||
|
expect(result).toHaveLength(1)
|
||||||
|
expect(result[0].id).toBe("a1")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("no assistant messages → returns empty array", () => {
|
||||||
|
const messages = [user("u1"), user("u2")]
|
||||||
|
const result = findAssistantMessages(messages, 0, "u1")
|
||||||
|
expect(result).toHaveLength(0)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("multiple assistant messages with matching parentID → all found", () => {
|
||||||
|
const messages = [user("u1"), assistant("a1", "u1"), assistant("a2", "u1")]
|
||||||
|
const result = findAssistantMessages(messages, 0, "u1")
|
||||||
|
expect(result).toHaveLength(2)
|
||||||
|
expect(result[0].id).toBe("a1")
|
||||||
|
expect(result[1].id).toBe("a2")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("does not return assistant messages with different parentID", () => {
|
||||||
|
const messages = [user("u1"), assistant("a1", "u1"), assistant("a2", "other")]
|
||||||
|
const result = findAssistantMessages(messages, 0, "u1")
|
||||||
|
expect(result).toHaveLength(1)
|
||||||
|
expect(result[0].id).toBe("a1")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("stops forward scan at next user message", () => {
|
||||||
|
const messages = [user("u1"), assistant("a1", "u1"), user("u2"), assistant("a2", "u1")]
|
||||||
|
const result = findAssistantMessages(messages, 0, "u1")
|
||||||
|
expect(result).toHaveLength(1)
|
||||||
|
expect(result[0].id).toBe("a1")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("stops backward scan at previous user message", () => {
|
||||||
|
const messages = [assistant("a0", "u1"), user("u0"), assistant("a1", "u1"), user("u1")]
|
||||||
|
const result = findAssistantMessages(messages, 3, "u1")
|
||||||
|
expect(result).toHaveLength(1)
|
||||||
|
expect(result[0].id).toBe("a1")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("invalid index returns empty array", () => {
|
||||||
|
const messages = [user("u1")]
|
||||||
|
expect(findAssistantMessages(messages, -1, "u1")).toHaveLength(0)
|
||||||
|
expect(findAssistantMessages(messages, 5, "u1")).toHaveLength(0)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
@ -1372,12 +1372,7 @@ NOTE: At any point in time through this workflow you should feel free to ask the
|
||||||
// Keep the loop running so tool results can be sent back to the model.
|
// Keep the loop running so tool results can be sent back to the model.
|
||||||
const hasToolCalls = lastAssistantMsg?.parts.some((part) => part.type === "tool") ?? false
|
const hasToolCalls = lastAssistantMsg?.parts.some((part) => part.type === "tool") ?? false
|
||||||
|
|
||||||
if (
|
if (!hasToolCalls && shouldExitLoop(lastUser, lastAssistant)) {
|
||||||
lastAssistant?.finish &&
|
|
||||||
!["tool-calls"].includes(lastAssistant.finish) &&
|
|
||||||
!hasToolCalls &&
|
|
||||||
lastUser.id < lastAssistant.id
|
|
||||||
) {
|
|
||||||
log.info("exiting loop", { sessionID })
|
log.info("exiting loop", { sessionID })
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
|
|
@ -1907,4 +1902,12 @@ NOTE: At any point in time through this workflow you should feel free to ask the
|
||||||
const argsRegex = /(?:\[Image\s+\d+\]|"[^"]*"|'[^']*'|[^\s"']+)/gi
|
const argsRegex = /(?:\[Image\s+\d+\]|"[^"]*"|'[^']*'|[^\s"']+)/gi
|
||||||
const placeholderRegex = /\$(\d+)/g
|
const placeholderRegex = /\$(\d+)/g
|
||||||
const quoteTrimRegex = /^["']|["']$/g
|
const quoteTrimRegex = /^["']|["']$/g
|
||||||
|
|
||||||
|
/** @internal Exported for testing */
|
||||||
|
export function shouldExitLoop(lastUser: MessageV2.User | undefined, lastAssistant: MessageV2.Assistant | undefined) {
|
||||||
|
if (!lastUser) return false
|
||||||
|
if (!lastAssistant?.finish) return false
|
||||||
|
if (["tool-calls", "unknown"].includes(lastAssistant.finish)) return false
|
||||||
|
return lastAssistant.parentID === lastUser.id
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,85 @@
|
||||||
|
import { describe, expect, test } from "bun:test"
|
||||||
|
import type { MessageV2 } from "../../src/session/message-v2"
|
||||||
|
import { SessionPrompt } from "../../src/session/prompt"
|
||||||
|
|
||||||
|
function makeUser(id: string): MessageV2.User {
|
||||||
|
return {
|
||||||
|
id,
|
||||||
|
role: "user",
|
||||||
|
sessionID: "session-1",
|
||||||
|
time: { created: Date.now() },
|
||||||
|
agent: "default",
|
||||||
|
model: { providerID: "openai", modelID: "gpt-4" },
|
||||||
|
} as MessageV2.User
|
||||||
|
}
|
||||||
|
|
||||||
|
function makeAssistant(
|
||||||
|
id: string,
|
||||||
|
parentID: string,
|
||||||
|
finish?: string,
|
||||||
|
): MessageV2.Assistant {
|
||||||
|
return {
|
||||||
|
id,
|
||||||
|
role: "assistant",
|
||||||
|
sessionID: "session-1",
|
||||||
|
parentID,
|
||||||
|
mode: "default",
|
||||||
|
agent: "default",
|
||||||
|
path: { cwd: "/tmp", root: "/tmp" },
|
||||||
|
cost: 0,
|
||||||
|
tokens: { input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } },
|
||||||
|
modelID: "gpt-4",
|
||||||
|
providerID: "openai",
|
||||||
|
time: { created: Date.now() },
|
||||||
|
finish,
|
||||||
|
} as MessageV2.Assistant
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("shouldExitLoop", () => {
|
||||||
|
test("normal case: user ID < assistant ID, parentID matches, finish=end_turn → exits", () => {
|
||||||
|
const user = makeUser("01AAA")
|
||||||
|
const assistant = makeAssistant("01BBB", "01AAA", "end_turn")
|
||||||
|
expect(SessionPrompt.shouldExitLoop(user, assistant)).toBe(true)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("clock skew: user ID > assistant ID, parentID matches, finish=stop → exits", () => {
|
||||||
|
// Simulates client clock ahead: user message ID sorts AFTER the assistant ID
|
||||||
|
const user = makeUser("01ZZZ")
|
||||||
|
const assistant = makeAssistant("01AAA", "01ZZZ", "stop")
|
||||||
|
expect(SessionPrompt.shouldExitLoop(user, assistant)).toBe(true)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("unfinished assistant: finish=tool-calls → does NOT exit", () => {
|
||||||
|
const user = makeUser("01AAA")
|
||||||
|
const assistant = makeAssistant("01BBB", "01AAA", "tool-calls")
|
||||||
|
expect(SessionPrompt.shouldExitLoop(user, assistant)).toBe(false)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("unfinished assistant: finish=unknown → does NOT exit", () => {
|
||||||
|
const user = makeUser("01AAA")
|
||||||
|
const assistant = makeAssistant("01BBB", "01AAA", "unknown")
|
||||||
|
expect(SessionPrompt.shouldExitLoop(user, assistant)).toBe(false)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("no assistant yet → does NOT exit", () => {
|
||||||
|
const user = makeUser("01AAA")
|
||||||
|
expect(SessionPrompt.shouldExitLoop(user, undefined)).toBe(false)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("assistant has no finish → does NOT exit", () => {
|
||||||
|
const user = makeUser("01AAA")
|
||||||
|
const assistant = makeAssistant("01BBB", "01AAA", undefined)
|
||||||
|
expect(SessionPrompt.shouldExitLoop(user, assistant)).toBe(false)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("parentID mismatch → does NOT exit", () => {
|
||||||
|
const user = makeUser("01AAA")
|
||||||
|
const assistant = makeAssistant("01BBB", "01OTHER", "end_turn")
|
||||||
|
expect(SessionPrompt.shouldExitLoop(user, assistant)).toBe(false)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("no user message → does NOT exit", () => {
|
||||||
|
const assistant = makeAssistant("01BBB", "01AAA", "end_turn")
|
||||||
|
expect(SessionPrompt.shouldExitLoop(undefined, assistant)).toBe(false)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
@ -0,0 +1,40 @@
|
||||||
|
import type { AssistantMessage, Message as MessageType } from "@opencode-ai/sdk/v2/client"
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Find assistant messages that are replies to a given user message.
|
||||||
|
*
|
||||||
|
* Scans forward from the user message index first, then falls back to scanning
|
||||||
|
* backward. The backward scan handles clock skew where assistant messages
|
||||||
|
* (generated server-side) sort before the user message (generated client-side
|
||||||
|
* with an ahead clock) in the ID-sorted array.
|
||||||
|
*/
|
||||||
|
export function findAssistantMessages(
|
||||||
|
messages: MessageType[],
|
||||||
|
userIndex: number,
|
||||||
|
userID: string,
|
||||||
|
): AssistantMessage[] {
|
||||||
|
if (userIndex < 0 || userIndex >= messages.length) return []
|
||||||
|
|
||||||
|
const result: AssistantMessage[] = []
|
||||||
|
|
||||||
|
// Scan forward from user message
|
||||||
|
for (let i = userIndex + 1; i < messages.length; i++) {
|
||||||
|
const item = messages[i]
|
||||||
|
if (!item) continue
|
||||||
|
if (item.role === "user") break
|
||||||
|
if (item.role === "assistant" && item.parentID === userID) result.push(item as AssistantMessage)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Scan backward to find assistant messages that sort before the user
|
||||||
|
// message due to clock skew between client and server
|
||||||
|
if (result.length === 0) {
|
||||||
|
for (let i = userIndex - 1; i >= 0; i--) {
|
||||||
|
const item = messages[i]
|
||||||
|
if (!item) continue
|
||||||
|
if (item.role === "user") break
|
||||||
|
if (item.role === "assistant" && item.parentID === userID) result.push(item as AssistantMessage)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return result
|
||||||
|
}
|
||||||
|
|
@ -14,6 +14,7 @@ import { createEffect, createMemo, createSignal, For, on, ParentProps, Show } fr
|
||||||
import { createStore } from "solid-js/store"
|
import { createStore } from "solid-js/store"
|
||||||
import { Dynamic } from "solid-js/web"
|
import { Dynamic } from "solid-js/web"
|
||||||
import { AssistantParts, Message, MessageDivider, PART_MAPPING, type UserActions } from "./message-part"
|
import { AssistantParts, Message, MessageDivider, PART_MAPPING, type UserActions } from "./message-part"
|
||||||
|
import { findAssistantMessages } from "./find-assistant-messages"
|
||||||
import { Card } from "./card"
|
import { Card } from "./card"
|
||||||
import { Accordion } from "./accordion"
|
import { Accordion } from "./accordion"
|
||||||
import { StickyAccordionHeader } from "./sticky-accordion-header"
|
import { StickyAccordionHeader } from "./sticky-accordion-header"
|
||||||
|
|
@ -270,14 +271,7 @@ export function SessionTurn(
|
||||||
const index = messageIndex()
|
const index = messageIndex()
|
||||||
if (index < 0) return emptyAssistant
|
if (index < 0) return emptyAssistant
|
||||||
|
|
||||||
const result: AssistantMessage[] = []
|
return findAssistantMessages(messages, index, msg.id)
|
||||||
for (let i = index + 1; i < messages.length; i++) {
|
|
||||||
const item = messages[i]
|
|
||||||
if (!item) continue
|
|
||||||
if (item.role === "user") break
|
|
||||||
if (item.role === "assistant" && item.parentID === msg.id) result.push(item as AssistantMessage)
|
|
||||||
}
|
|
||||||
return result
|
|
||||||
},
|
},
|
||||||
emptyAssistant,
|
emptyAssistant,
|
||||||
{ equals: same },
|
{ equals: same },
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue