mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 16:47:34 +00:00
fix(native-chat): retire a first send's echo in transcript time, not this client's (#27099)
* fix(native-chat): retire a first send's echo in transcript time, not this client's A send into an empty transcript had no row to anchor on, so its echo was matched against rows newer than the renderer's send time. Row timestamps come from the host that writes the transcript; with that host's clock behind, the echo's own turn looked older than the send and the queued bubble never retired. A send made after the transcript read has settled empty now records that every row is after it. A send made while the read is still loading keeps the existing rule, so older history arriving later cannot retire it. * fix(native-chat): bind the empty-transcript claim to the session that was read 'ready' with no rows is also the placeholder before a provider session is known, which read nothing; a resumed transcript can then hold an older identical turn that retired the new send, and with it a rejected or unconfirmed notice. The claim now needs a provider session id, records it, and is released for any other session, falling back to the existing rule. --------- Co-authored-by: m4air <m4air@m4airs-MacBook-Air.local>
This commit is contained in:
@@ -157,7 +157,7 @@ export function NativeChatResolvedView({
|
||||
() => ({ paneKey, agent, sessionId }),
|
||||
[paneKey, agent, sessionId]
|
||||
)
|
||||
const delivery = useNativeChatPendingDelivery({ paneKey, agent, messages: session.messages })
|
||||
const delivery = useNativeChatPendingDelivery({ paneKey, agent, session })
|
||||
const { pending, record, clear } = delivery
|
||||
// Slash commands aren't chat turns, so they get a small local "Ran /clear"
|
||||
// system line instead of a user bubble. Capped + cached per conversation.
|
||||
|
||||
@@ -514,6 +514,27 @@ describe('pendingSendsAsMessages', () => {
|
||||
expect(prunePendingSends(pending, history)).toEqual(pending)
|
||||
})
|
||||
|
||||
// #11519: a first send into a settled empty transcript has no row to compare, and the
|
||||
// host that writes the transcript may run a clock behind this one.
|
||||
it('retires a send into a settled empty transcript whatever the host clock says', () => {
|
||||
const fiveMinutes = 5 * 60_000
|
||||
const pending = [
|
||||
{
|
||||
...pendingOf('first-send', 'run tests'),
|
||||
sentAt: 1_000_000,
|
||||
afterMessageId: null,
|
||||
afterEmptyTranscriptSessionId: 'session-1'
|
||||
}
|
||||
]
|
||||
const hostBehind = [
|
||||
{ ...userMessage('u1', 'run tests'), timestamp: 1_000_000 - fiveMinutes },
|
||||
{ ...assistantMessage('a1', 'passed'), timestamp: 1_000_000 - fiveMinutes + 10 }
|
||||
]
|
||||
|
||||
expect(pendingSendsAsMessages(pending, hostBehind)).toEqual([])
|
||||
expect(prunePendingSends(pending, hostBehind)).toEqual([])
|
||||
})
|
||||
|
||||
it('uses the transcript boundary clock after pagination, not the renderer send clock', () => {
|
||||
const pending = [
|
||||
{
|
||||
|
||||
@@ -40,6 +40,9 @@ export type NativeChatPendingSend = {
|
||||
afterMessageId?: string | null
|
||||
/** Timestamp of that boundary in the transcript host's clock domain. */
|
||||
afterMessageTimestamp?: number | null
|
||||
/** The provider session whose transcript had been read empty when this was sent, so every
|
||||
* row of that session is after it. Unset for a read still loading or no session yet. */
|
||||
afterEmptyTranscriptSessionId?: string
|
||||
/** 1-based occurrence among identical sends sharing the same boundary. */
|
||||
matchingOccurrence?: number
|
||||
/** Shared time boundary when that message boundary is unavailable. */
|
||||
@@ -106,7 +109,10 @@ function messagesAfterPendingBoundary(
|
||||
return messages
|
||||
}
|
||||
if (pending.afterMessageId === null) {
|
||||
return messages.filter((message) => messageIsAfterPendingTimestamp(message, pending))
|
||||
// Why: the send time is this client's clock and row timestamps are the host's (#11519).
|
||||
return pending.afterEmptyTranscriptSessionId
|
||||
? messages
|
||||
: messages.filter((message) => messageIsAfterPendingTimestamp(message, pending))
|
||||
}
|
||||
const boundaryIndex = messages.findIndex((message) => message.id === pending.afterMessageId)
|
||||
if (boundaryIndex !== -1) {
|
||||
@@ -117,6 +123,35 @@ function messagesAfterPendingBoundary(
|
||||
return messages.filter((message) => messageIsAfterPendingTimestamp(message, pending))
|
||||
}
|
||||
|
||||
/**
|
||||
* Drop the empty-transcript claim from sends made in another provider session: that claim
|
||||
* describes one transcript, and a resumed one can hold an older identical turn.
|
||||
*/
|
||||
export function releaseEmptyTranscriptClaims(
|
||||
pending: NativeChatPendingSend[],
|
||||
sessionId: string | null
|
||||
): NativeChatPendingSend[] {
|
||||
if (
|
||||
!pending.some(
|
||||
(entry) =>
|
||||
entry.afterEmptyTranscriptSessionId !== undefined &&
|
||||
entry.afterEmptyTranscriptSessionId !== sessionId
|
||||
)
|
||||
) {
|
||||
return pending
|
||||
}
|
||||
return pending.map((entry) => {
|
||||
if (
|
||||
entry.afterEmptyTranscriptSessionId === undefined ||
|
||||
entry.afterEmptyTranscriptSessionId === sessionId
|
||||
) {
|
||||
return entry
|
||||
}
|
||||
const { afterEmptyTranscriptSessionId: _released, ...rest } = entry
|
||||
return rest
|
||||
})
|
||||
}
|
||||
|
||||
function messageIsAfterPendingTimestamp(
|
||||
message: NativeChatMessage,
|
||||
pending: NativeChatPendingSend
|
||||
|
||||
@@ -15,14 +15,17 @@ const boundary: NativeChatMessage = {
|
||||
timestamp: null,
|
||||
source: 'transcript'
|
||||
}
|
||||
const args = { paneKey: 'pane', agent: 'claude' as const, messages: [boundary] }
|
||||
const args = { paneKey: 'pane', agent: 'claude' as const }
|
||||
function userRow(text: string): NativeChatMessage {
|
||||
return { ...boundary, id: `user:${text}`, role: 'user', blocks: [{ type: 'text', text }] }
|
||||
}
|
||||
function render() {
|
||||
return renderHook(({ messages }) => useNativeChatPendingDelivery({ ...args, messages }), {
|
||||
initialProps: { messages: [boundary] }
|
||||
})
|
||||
return renderHook(
|
||||
({ messages }) => useNativeChatPendingDelivery({ ...args, session: { messages } }),
|
||||
{
|
||||
initialProps: { messages: [boundary] }
|
||||
}
|
||||
)
|
||||
}
|
||||
async function tick(ms: number) {
|
||||
await act(async () => {
|
||||
@@ -39,6 +42,51 @@ afterEach(() => {
|
||||
})
|
||||
|
||||
describe('terminal Chat pending delivery', () => {
|
||||
it('records a send into a read empty transcript as after every row of that session', () => {
|
||||
const { result } = renderHook(() =>
|
||||
useNativeChatPendingDelivery({
|
||||
...args,
|
||||
session: { messages: [], readPhase: 'ready', sessionId: 'session-1' }
|
||||
})
|
||||
)
|
||||
act(() => result.current.record('first'))
|
||||
expect(result.current.pending[0]).toMatchObject({
|
||||
afterMessageId: null,
|
||||
afterEmptyTranscriptSessionId: 'session-1'
|
||||
})
|
||||
})
|
||||
it.each([
|
||||
['still loading', { readPhase: 'loading' as const, sessionId: 'session-1' }],
|
||||
['no session yet', { readPhase: 'ready' as const, sessionId: null }]
|
||||
])('does not claim an empty transcript while %s', (_label, session) => {
|
||||
const { result } = renderHook(() =>
|
||||
useNativeChatPendingDelivery({ ...args, session: { messages: [], ...session } })
|
||||
)
|
||||
act(() => result.current.record('first'))
|
||||
expect(result.current.pending[0]?.afterEmptyTranscriptSessionId).toBeUndefined()
|
||||
})
|
||||
// A resumed transcript can hold an older identical turn; the claim was about another one.
|
||||
it('keeps a send, and its failure notice, when a different session with old history loads', () => {
|
||||
const old: NativeChatMessage[] = [
|
||||
{ ...userRow('run tests'), id: 'old-user', timestamp: 10 },
|
||||
{ ...boundary, id: 'old-answer', timestamp: 20 }
|
||||
]
|
||||
const initialSession: Parameters<typeof useNativeChatPendingDelivery>[0]['session'] = {
|
||||
messages: [],
|
||||
readPhase: 'ready',
|
||||
sessionId: 'new-session'
|
||||
}
|
||||
const { result, rerender } = renderHook(
|
||||
({ session }) => useNativeChatPendingDelivery({ ...args, session }),
|
||||
{ initialProps: { session: initialSession } }
|
||||
)
|
||||
act(() => {
|
||||
result.current.reject(result.current.record('run tests'))
|
||||
})
|
||||
rerender({ session: { messages: old, readPhase: 'ready', sessionId: 'resumed-session' } })
|
||||
expect(result.current.pending.map((entry) => entry.text)).toEqual(['run tests'])
|
||||
expect(result.current.notices.size).toBe(1)
|
||||
})
|
||||
it('never flags an ordinary send, such as one Claude queues mid-turn', async () => {
|
||||
const { result } = render()
|
||||
act(() => result.current.record('queued follow up'))
|
||||
|
||||
@@ -7,10 +7,12 @@ import {
|
||||
pendingSendsAsMessages,
|
||||
prunePendingSends,
|
||||
readPendingSendCache,
|
||||
releaseEmptyTranscriptClaims,
|
||||
writePendingSendCache,
|
||||
type NativeChatPendingSend
|
||||
} from './native-chat-pending'
|
||||
import type { NativeChatDeliveryNotice } from './NativeChatMessageRow'
|
||||
import type { ReadState } from './native-chat-live-session-contract'
|
||||
|
||||
/** How long a send whose write acknowledgment was lost waits for its row; the phone's hold matches. */
|
||||
export const NATIVE_CHAT_UNCONFIRMED_SEND_HOLD_MS = 20_000
|
||||
@@ -21,11 +23,25 @@ const NO_NOTICES: ReadonlyMap<string, NativeChatDeliveryNotice> = new Map()
|
||||
export function useNativeChatPendingDelivery(args: {
|
||||
paneKey: string
|
||||
agent: AgentType
|
||||
messages: NativeChatMessage[]
|
||||
/** `readPhase` 'ready' with a `sessionId` means an empty `messages` was a real read. */
|
||||
session: {
|
||||
messages: NativeChatMessage[]
|
||||
readPhase?: ReadState['phase']
|
||||
sessionId?: string | null
|
||||
}
|
||||
}) {
|
||||
const { paneKey, agent, messages } = args
|
||||
const { paneKey, agent } = args
|
||||
const { messages } = args.session
|
||||
const sessionId = args.session.sessionId ?? null
|
||||
// Why sessionId too: with none, 'ready' is a placeholder that read nothing.
|
||||
const readEmptySessionId =
|
||||
args.session.readPhase === 'ready' && messages.length === 0 ? sessionId : null
|
||||
const scope = useMemo(() => ({ paneKey, agent }), [paneKey, agent])
|
||||
const [pending, setPending] = useState(() => readPendingSendCache(scope))
|
||||
const [cachedPending, setPending] = useState(() => readPendingSendCache(scope))
|
||||
const pending = useMemo(
|
||||
() => releaseEmptyTranscriptClaims(cachedPending, sessionId),
|
||||
[cachedPending, sessionId]
|
||||
)
|
||||
useEffect(() => setPending(readPendingSendCache(scope)), [scope])
|
||||
const save = useCallback(
|
||||
(update: (entries: NativeChatPendingSend[]) => NativeChatPendingSend[]) => {
|
||||
@@ -39,8 +55,8 @@ export function useNativeChatPendingDelivery(args: {
|
||||
[scope]
|
||||
)
|
||||
useEffect(() => {
|
||||
save((entries) => prunePendingSends(entries, messages))
|
||||
}, [messages, save])
|
||||
save((entries) => prunePendingSends(releaseEmptyTranscriptClaims(entries, sessionId), messages))
|
||||
}, [messages, save, sessionId])
|
||||
const record = useCallback(
|
||||
(text: string, imagePaths?: string[]) => {
|
||||
const sentAt = Date.now()
|
||||
@@ -51,12 +67,13 @@ export function useNativeChatPendingDelivery(args: {
|
||||
sentAt,
|
||||
afterMessageId: boundary?.id ?? null,
|
||||
afterMessageTimestamp: boundary?.timestamp ?? null,
|
||||
...(readEmptySessionId ? { afterEmptyTranscriptSessionId: readEmptySessionId } : {}),
|
||||
...(imagePaths ? { imagePaths } : {})
|
||||
}
|
||||
setPending(appendPendingSendCache(scope, entry))
|
||||
return entry.id
|
||||
},
|
||||
[messages, scope]
|
||||
[messages, readEmptySessionId, scope]
|
||||
)
|
||||
const cancel = useCallback(
|
||||
(id: string) => save((entries) => entries.filter((entry) => entry.id !== id)),
|
||||
@@ -114,7 +131,9 @@ export function useNativeChatPendingDelivery(args: {
|
||||
return entries
|
||||
}
|
||||
const unmatched = new Set(
|
||||
pendingSendsAsMessages(due, messages).map((message) => message.id)
|
||||
pendingSendsAsMessages(releaseEmptyTranscriptClaims(due, sessionId), messages).map(
|
||||
(message) => message.id
|
||||
)
|
||||
)
|
||||
if (unmatched.size === 0) {
|
||||
return entries
|
||||
@@ -129,7 +148,7 @@ export function useNativeChatPendingDelivery(args: {
|
||||
Math.max(0, nextHoldDeadline - Date.now())
|
||||
)
|
||||
return () => clearTimeout(timer)
|
||||
}, [nextHoldDeadline, messages, save])
|
||||
}, [nextHoldDeadline, messages, save, sessionId])
|
||||
|
||||
const notices = useMemo(() => {
|
||||
if (!pending.some((entry) => entry.delivery)) {
|
||||
|
||||
Reference in New Issue
Block a user