mirror of
https://github.com/stablyai/orca.git
synced 2026-10-22 08:03:37 +00:00
fix(mobile-native-chat): stop the hydration rebase stranding a send on its own echo
Rebasing recounted the send's ordinal against the first authoritative read. That read can already carry the send's own echo — a re-subscribe after a tab switch or reconnect returns whatever exists now — so the ordinal landed one past anything the transcript could supply. The bubble never cleared, it stayed a live segment at the head of its run so no later pair could glue either, and `earlierOutstanding` carried the inflation onto the next send of the same text. Only the tail needs recovering; the ordinal was already counted against an empty transcript, which is right for "no history was known". A caption-less image echo keeps its captured tail, since it counts turns after it. `baselineResolved` also has to mean "captured against a settled read", not merely "not loading": a read that failed hands back an empty list that reads as an empty conversation, and the null tail then let any row the successful read finally brought glue-retire those sends.
This commit is contained in:
@@ -41,15 +41,16 @@ describe('rebaseMobileNativeChatPendingBaselines', () => {
|
||||
expect(rebaseMobileNativeChatPendingBaselines(history, pending)).toBe(pending)
|
||||
})
|
||||
|
||||
it('pins an unresolved send to the loaded tail and past the rows it never saw', () => {
|
||||
it('gives an unresolved send the loaded tail as its glue boundary', () => {
|
||||
expect(
|
||||
rebaseMobileNativeChatPendingBaselines(history, [unresolved('p1', 'run the tests')])
|
||||
).toEqual([
|
||||
{
|
||||
id: 'p1',
|
||||
text: 'run the tests',
|
||||
// The one row already in history is not this send's echo; it waits for the second.
|
||||
expectedOccurrence: 2,
|
||||
// NOT recounted against this read: the read may already carry this send's
|
||||
// own echo, and counting it as history would strand the bubble forever.
|
||||
expectedOccurrence: 1,
|
||||
baselineTailMessageId: 'm2',
|
||||
baselineResolved: true
|
||||
}
|
||||
@@ -68,23 +69,16 @@ describe('rebaseMobileNativeChatPendingBaselines', () => {
|
||||
])
|
||||
})
|
||||
|
||||
it('gives identical queued sends consecutive ordinals', () => {
|
||||
it('keeps the ordinals the queue already assigned', () => {
|
||||
expect(
|
||||
rebaseMobileNativeChatPendingBaselines(history, [
|
||||
unresolved('p1', 'run the tests'),
|
||||
unresolved('p2', 'run the tests')
|
||||
unresolved('p1', 'run the tests', 1),
|
||||
unresolved('p2', 'run the tests', 2)
|
||||
]).map((item) => item.expectedOccurrence)
|
||||
).toEqual([2, 3])
|
||||
).toEqual([1, 2])
|
||||
})
|
||||
|
||||
it('normalizes whitespace when counting a send against the loaded rows', () => {
|
||||
expect(
|
||||
rebaseMobileNativeChatPendingBaselines(history, [unresolved('p1', ' run the tests \n')])[0]
|
||||
?.expectedOccurrence
|
||||
).toBe(2)
|
||||
})
|
||||
|
||||
it('leaves already-resolved neighbours untouched but still counts them', () => {
|
||||
it('leaves already-resolved neighbours untouched', () => {
|
||||
const resolved: MobileNativeChatPendingMessage = {
|
||||
id: 'p1',
|
||||
text: 'run the tests',
|
||||
@@ -94,19 +88,28 @@ describe('rebaseMobileNativeChatPendingBaselines', () => {
|
||||
}
|
||||
const rebased = rebaseMobileNativeChatPendingBaselines(history, [
|
||||
resolved,
|
||||
unresolved('p2', 'run the tests')
|
||||
unresolved('p2', 'run the tests', 2)
|
||||
])
|
||||
expect(rebased[0]).toBe(resolved)
|
||||
expect(rebased[1]?.expectedOccurrence).toBe(3)
|
||||
expect(rebased[1]).toEqual({
|
||||
id: 'p2',
|
||||
text: 'run the tests',
|
||||
expectedOccurrence: 2,
|
||||
baselineTailMessageId: 'm2',
|
||||
baselineResolved: true
|
||||
})
|
||||
})
|
||||
|
||||
it('ordinals caption-less image echoes among themselves, not against text rows', () => {
|
||||
// An image echo counts image turns AFTER its tail, so moving the tail onto this
|
||||
// read would discard an echo the read already carries.
|
||||
it('keeps a caption-less image echo on its captured tail', () => {
|
||||
const images = { ...unresolved('p1', ''), images: ['file:///a.png'] }
|
||||
const rebased = rebaseMobileNativeChatPendingBaselines(history, [
|
||||
images,
|
||||
{ ...unresolved('p2', ''), images: ['file:///b.png'] }
|
||||
{ ...unresolved('p2', '', 2), images: ['file:///b.png'] }
|
||||
])
|
||||
expect(rebased.map((item) => item.expectedOccurrence)).toEqual([1, 2])
|
||||
expect(rebased.map((item) => item.baselineTailMessageId)).toEqual(['m2', 'm2'])
|
||||
expect(rebased.map((item) => item.baselineTailMessageId)).toEqual([null, null])
|
||||
expect(rebased.every((item) => item.baselineResolved)).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,27 +1,29 @@
|
||||
import type { NativeChatMessage } from '../../../src/shared/native-chat-types'
|
||||
import {
|
||||
countUserTextOccurrences,
|
||||
normalizeReconcileText
|
||||
} from './mobile-native-chat-draft-reconcile'
|
||||
import { normalizeReconcileText } from './mobile-native-chat-draft-reconcile'
|
||||
import type { MobileNativeChatPendingMessage } from './mobile-native-chat-pending-echo'
|
||||
|
||||
/**
|
||||
* Pin the sends issued while the transcript was still hydrating onto the first
|
||||
* authoritative read of this session's history.
|
||||
* Give the sends issued before this session's history was known the boundary
|
||||
* they never had, on the first authoritative read.
|
||||
*
|
||||
* Such a send has no usable baseline at capture time: `messages` was empty, or
|
||||
* still the previously active tab's, so both its tail and its occurrence count
|
||||
* describe somebody else's transcript. Rebasing them here — rather than marking
|
||||
* the send permanently untrustworthy, which stranded it as a queued bubble and
|
||||
* as a glue barrier for its neighbours for the rest of the session — leaves it
|
||||
* matching exactly the rows that arrive from now on.
|
||||
* Such a send saw no transcript, so it has no row it can judge candidates
|
||||
* against — which held it out of matching entirely, stranding it as a queued
|
||||
* bubble and, worse, as a permanent glue barrier for its neighbours. Recovering
|
||||
* the tail is all that is needed: the ordinal was already counted against an
|
||||
* empty transcript, which is exactly right for "no history was known".
|
||||
*
|
||||
* Residual: if this read was taken AFTER the send already echoed — a re-subscribe
|
||||
* on tab switch or reconnect, when the first load never completed — its own echo
|
||||
* is counted as history and the bubble waits for a row that will not come. The
|
||||
* transport writes keystrokes into a TUI, so there is no client message id to
|
||||
* tell the two apart; the send is bounded instead by serializing submits per PTY
|
||||
* so the glue never forms.
|
||||
* The ordinal is deliberately NOT recounted against this read. The read can
|
||||
* already carry the send's own echo — a re-subscribe after a tab switch or a
|
||||
* reconnect returns whatever exists now — and nothing local separates that echo
|
||||
* from an identical older prompt: the transport writes keystrokes into a TUI and
|
||||
* carries no message id, and row timestamps come from the host while the send
|
||||
* time comes from the phone. Counting it as history puts the ordinal one past
|
||||
* anything the transcript can supply, which strands the bubble for the rest of
|
||||
* the session and stops its whole run from ever gluing.
|
||||
*
|
||||
* A caption-less image echo keeps its captured tail: it reconciles by counting
|
||||
* image turns AFTER that tail, so pinning the tail past this read would throw
|
||||
* away an echo the read already carries.
|
||||
*/
|
||||
export function rebaseMobileNativeChatPendingBaselines(
|
||||
messages: readonly NativeChatMessage[],
|
||||
@@ -31,29 +33,12 @@ export function rebaseMobileNativeChatPendingBaselines(
|
||||
return current
|
||||
}
|
||||
const baselineTailMessageId = messages.at(-1)?.id ?? null
|
||||
const earlierByText = new Map<string, number>()
|
||||
let earlierImageEchoes = 0
|
||||
return current.map((item) => {
|
||||
const normalized = normalizeReconcileText(item.text)
|
||||
const earlier = normalized === '' ? earlierImageEchoes : (earlierByText.get(normalized) ?? 0)
|
||||
if (normalized === '') {
|
||||
earlierImageEchoes += item.images?.length ? 1 : 0
|
||||
} else {
|
||||
earlierByText.set(normalized, earlier + 1)
|
||||
}
|
||||
if (item.baselineResolved) {
|
||||
return item
|
||||
}
|
||||
return {
|
||||
...item,
|
||||
baselineTailMessageId,
|
||||
baselineResolved: true,
|
||||
// Ordinals stay relative to the queue: earlier still-pending sends of the
|
||||
// same text claim the earlier rows, so this one waits for its own.
|
||||
expectedOccurrence:
|
||||
normalized === ''
|
||||
? earlier + 1
|
||||
: countUserTextOccurrences(messages, normalized) + earlier + 1
|
||||
}
|
||||
return item.images?.length || normalizeReconcileText(item.text) === ''
|
||||
? { ...item, baselineResolved: true }
|
||||
: { ...item, baselineResolved: true, baselineTailMessageId }
|
||||
})
|
||||
}
|
||||
|
||||
@@ -111,7 +111,8 @@ export function useMobileNativeChatController(args: {
|
||||
// a null is indistinguishable from a host retraction, and peeking at the
|
||||
// terminal view would permanently decline the prefill.
|
||||
chatActive: showNativeChat,
|
||||
transcriptLoading: nativeChatSession.transcriptLoading
|
||||
transcriptLoading: nativeChatSession.transcriptLoading,
|
||||
transcriptSettled: nativeChatSession.status === 'ready'
|
||||
})
|
||||
|
||||
const nativeChatStatus = activeChatResolution ? activeSessionTab?.agentStatus : null
|
||||
|
||||
@@ -52,10 +52,12 @@ describe('useMobileNativeChatDrafts glued pending sends', () => {
|
||||
|
||||
function Harness({
|
||||
messages,
|
||||
transcriptLoading = false
|
||||
transcriptLoading = false,
|
||||
transcriptSettled = !transcriptLoading
|
||||
}: {
|
||||
messages: NativeChatMessage[]
|
||||
transcriptLoading?: boolean
|
||||
transcriptSettled?: boolean
|
||||
}): null {
|
||||
state = useMobileNativeChatDrafts({
|
||||
hostId: 'host',
|
||||
@@ -63,19 +65,30 @@ describe('useMobileNativeChatDrafts glued pending sends', () => {
|
||||
tabId: 'tab',
|
||||
sessionId: 'session',
|
||||
messages,
|
||||
transcriptLoading
|
||||
transcriptLoading,
|
||||
transcriptSettled
|
||||
})
|
||||
return null
|
||||
}
|
||||
|
||||
async function mount(messages: NativeChatMessage[], transcriptLoading = false): Promise<void> {
|
||||
async function mount(
|
||||
messages: NativeChatMessage[],
|
||||
transcriptLoading = false,
|
||||
transcriptSettled = !transcriptLoading
|
||||
): Promise<void> {
|
||||
await act(async () => {
|
||||
renderer = create(createElement(Harness, { messages, transcriptLoading }))
|
||||
renderer = create(createElement(Harness, { messages, transcriptLoading, transcriptSettled }))
|
||||
})
|
||||
}
|
||||
|
||||
async function update(messages: NativeChatMessage[], transcriptLoading = false): Promise<void> {
|
||||
await act(async () => renderer?.update(createElement(Harness, { messages, transcriptLoading })))
|
||||
async function update(
|
||||
messages: NativeChatMessage[],
|
||||
transcriptLoading = false,
|
||||
transcriptSettled = !transcriptLoading
|
||||
): Promise<void> {
|
||||
await act(async () =>
|
||||
renderer?.update(createElement(Harness, { messages, transcriptLoading, transcriptSettled }))
|
||||
)
|
||||
}
|
||||
|
||||
function send(text: string): void {
|
||||
@@ -213,17 +226,71 @@ describe('useMobileNativeChatDrafts glued pending sends', () => {
|
||||
expect(state?.pending).toEqual([])
|
||||
})
|
||||
|
||||
it('does not retire a hydration-time send against history it never saw', async () => {
|
||||
// The read that resolves a hydration-time send can already contain that
|
||||
// send's own echo — a re-subscribe returns whatever exists now — and nothing
|
||||
// local tells that echo from an identical older prompt. Claiming the row is
|
||||
// the safe direction: being wrong costs one round trip of invisibility,
|
||||
// while holding costs a queued bubble that never clears and a run that can
|
||||
// never glue again.
|
||||
it('retires a hydration-time send whose echo arrives with the read', async () => {
|
||||
await mount([], true)
|
||||
act(() => send('run the tests'))
|
||||
|
||||
// The read lands carrying an older identical prompt: not this send's echo.
|
||||
const echoed = [userTurn('m1', 'run the tests', 1000), assistantTurn('m2', 'passed', 1100)]
|
||||
await update(echoed)
|
||||
expect(state?.pending).toEqual([])
|
||||
})
|
||||
|
||||
// The same read seen one reconnect later: the send left, the agent echoed it,
|
||||
// and only then did the first authoritative read land.
|
||||
it('retires a hydration-time send after a reconnect delivered the read late', async () => {
|
||||
await mount([], true)
|
||||
act(() => send('run the tests'))
|
||||
// Still loading: the first read never arrived before the drop.
|
||||
await update([], true)
|
||||
|
||||
const afterReconnect = [
|
||||
userTurn('m1', 'run the tests', 5000),
|
||||
assistantTurn('m2', 'ok', 5100)
|
||||
]
|
||||
await update(afterReconnect)
|
||||
expect(state?.pending).toEqual([])
|
||||
|
||||
// Nothing later resurrects it, and the run stays glue-capable.
|
||||
await update([...afterReconnect, assistantTurn('m3', 'all green', 6000)])
|
||||
expect(state?.pending).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
// A read that failed is not a boundary: `messages` is empty because the history
|
||||
// is unknown, not because the conversation is. Without that distinction the
|
||||
// empty list reads as "the transcript was empty", and any row the successful
|
||||
// read finally brings can claim these sends.
|
||||
describe('sends issued before any read settled', () => {
|
||||
it('keeps both bubbles when the late read carries only an older glue-alike', async () => {
|
||||
await mount([], false, false)
|
||||
rapidSend('fix the', 'bug')
|
||||
|
||||
const olderIdentical = [
|
||||
userTurn('m1', 'run the tests', 1000),
|
||||
assistantTurn('m2', 'passed', 1100)
|
||||
userTurn('m1', 'fix the bug', 1000),
|
||||
assistantTurn('m2', 'fixed', 1100)
|
||||
]
|
||||
await update(olderIdentical)
|
||||
expect(pendingTexts()).toEqual(['run the tests'])
|
||||
expect(pendingTexts()).toEqual(['fix the', 'bug'])
|
||||
expect(renderedPendingTexts(olderIdentical, state)).toEqual(['fix the', 'bug'])
|
||||
})
|
||||
|
||||
it('still retires them once their own glued row lands', async () => {
|
||||
await mount([], false, false)
|
||||
rapidSend('fix the', 'bug')
|
||||
const olderIdentical = [
|
||||
userTurn('m1', 'fix the bug', 1000),
|
||||
assistantTurn('m2', 'fixed', 1100)
|
||||
]
|
||||
await update(olderIdentical)
|
||||
|
||||
await update([...olderIdentical, userTurn('m3', 'fix the bug', 5000)])
|
||||
expect(state?.pending).toEqual([])
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -47,6 +47,10 @@ export function useMobileNativeChatDrafts(args: {
|
||||
* transcript still belongs to the previously active tab), so it cannot be
|
||||
* trusted to decline or retire the seed. */
|
||||
transcriptLoading?: boolean
|
||||
/** `messages` is this session's own settled history — so an empty one really
|
||||
* is an empty conversation, not a read that failed or never ran. Only then
|
||||
* does a send's captured tail describe a real boundary. */
|
||||
transcriptSettled?: boolean
|
||||
}): {
|
||||
composerText: string
|
||||
setComposerText: Dispatch<SetStateAction<string>>
|
||||
@@ -80,7 +84,8 @@ export function useMobileNativeChatDrafts(args: {
|
||||
launchDraft,
|
||||
launchDraftCreatedAt,
|
||||
chatActive = true,
|
||||
transcriptLoading
|
||||
transcriptLoading,
|
||||
transcriptSettled = !transcriptLoading
|
||||
} = args
|
||||
const draftKey = mobileNativeChatScopeKey(hostId, worktreeId, tabId)
|
||||
const pendingKey = draftKey && sessionId ? `${draftKey}\0${sessionId}` : null
|
||||
@@ -139,12 +144,13 @@ export function useMobileNativeChatDrafts(args: {
|
||||
normalizedText,
|
||||
baselineOccurrences: countUserTextOccurrences(messagesRef.current, normalizedText),
|
||||
baselineTailMessageId: messagesRef.current.at(-1)?.id ?? null,
|
||||
// A hydrating transcript is not yet this session's, so this baseline is
|
||||
// a placeholder the first authoritative read rebases.
|
||||
baselineResolved: !transcriptLoading
|
||||
// Only a settled read makes this a boundary. Anything else — hydrating,
|
||||
// or a read that failed — hands back an empty list that reads as "the
|
||||
// conversation was empty", which lets any row claim this send later.
|
||||
baselineResolved: transcriptSettled
|
||||
}
|
||||
},
|
||||
[draftKey, pendingKey, transcriptLoading]
|
||||
[draftKey, pendingKey, transcriptSettled]
|
||||
)
|
||||
|
||||
// Why: over relay the send RPC can take seconds (or lose only its ack), and a
|
||||
@@ -294,11 +300,11 @@ export function useMobileNativeChatDrafts(args: {
|
||||
}
|
||||
setPendingBySession((previous) => {
|
||||
const current = previous[pendingKey] ?? []
|
||||
// Rebase before retiring: a hydration-time send has to own a real boundary
|
||||
// before any row can be judged against it.
|
||||
const rebased = transcriptLoading
|
||||
? current
|
||||
: rebaseMobileNativeChatPendingBaselines(messages, current)
|
||||
// Rebase before retiring: a send captured before the history was known has
|
||||
// to own a real boundary before any row can be judged against it.
|
||||
const rebased = transcriptSettled
|
||||
? rebaseMobileNativeChatPendingBaselines(messages, current)
|
||||
: current
|
||||
const next = retireLandedMobileNativeChatPending(messages, rebased, landedImagePendingIds)
|
||||
if (next === current) {
|
||||
return previous
|
||||
@@ -310,7 +316,7 @@ export function useMobileNativeChatDrafts(args: {
|
||||
delete remaining[pendingKey]
|
||||
return remaining
|
||||
})
|
||||
}, [messages, pending, pendingKey, transcriptLoading])
|
||||
}, [messages, pending, pendingKey, transcriptSettled])
|
||||
|
||||
return {
|
||||
composerText: draftKey ? (drafts[draftKey] ?? '') : '',
|
||||
|
||||
Reference in New Issue
Block a user