fix(desktop): keep the 4033 re-run flag off the public callbacks; an unseen later turn stops an edit
Delta review 2 Lows: - The internal answeredFor parameter rode on the public onReload/onEdit callbacks, and assistant-ui calls onReload(parentId, config), so it received the config object on every reload. The 4033 re-run now goes through private runReload/runEdit, and the public callbacks take only their own arguments. - A deep edit re-aimed after the confirm accepted a later user turn that landed during the dialog, which archived a turn the user never answered for. revalidateEditPlan now refuses when the count of later user turns changed. Regenerate already asked again in this case.
This commit is contained in:
@@ -550,7 +550,8 @@ export function useSessionTileActions({ requestGateway, runtimeId, scope, stored
|
||||
[update]
|
||||
)
|
||||
|
||||
const reloadFromMessage = useCallback(
|
||||
// answeredFor is internal (the 4033 re-run); assistant-ui calls onReload(parentId, config).
|
||||
const runReload = useCallback(
|
||||
async (parentId: string | null, answeredFor?: ChatMessage[]): Promise<void> => {
|
||||
const state = readState()
|
||||
const runtimeId = runtimeIdRef.current
|
||||
@@ -609,15 +610,16 @@ export function useSessionTileActions({ requestGateway, runtimeId, scope, stored
|
||||
} else if ((await confirmDeep()) && runtimeIdRef.current === runtimeId) {
|
||||
// Re-run from the top: the session is re-read and re-validated after this wait. The answer
|
||||
// belongs to this runtime: a rebind during the dialog drops it.
|
||||
await reloadFromMessageRef.current(parentId, messages)
|
||||
await runReloadRef.current(parentId, messages)
|
||||
}
|
||||
}
|
||||
},
|
||||
[applySurvivorRowIds, copy.regenerateFailed, readState, submitRewind, t, update]
|
||||
)
|
||||
|
||||
const reloadFromMessageRef = useRef(reloadFromMessage)
|
||||
reloadFromMessageRef.current = reloadFromMessage
|
||||
const runReloadRef = useRef(runReload)
|
||||
runReloadRef.current = runReload
|
||||
const reloadFromMessage = useCallback((parentId: string | null) => runReload(parentId), [runReload])
|
||||
|
||||
const restoreToMessage = useCallback(
|
||||
async (messageId: string, target?: { text?: string; userOrdinal?: number | null }) => {
|
||||
@@ -708,7 +710,7 @@ export function useSessionTileActions({ requestGateway, runtimeId, scope, stored
|
||||
[applySurvivorRowIds, readMessages, readState, submitRewind, update]
|
||||
)
|
||||
|
||||
const editMessage = useCallback(
|
||||
const runEdit = useCallback(
|
||||
async (edited: AppendMessage, answeredFor?: ChatMessage[]): Promise<void> => {
|
||||
const planning = readMessages()
|
||||
const confirmDeep = () => confirm(deepCutConfirmRequest(t.assistant.thread))
|
||||
@@ -722,7 +724,7 @@ export function useSessionTileActions({ requestGateway, runtimeId, scope, stored
|
||||
|
||||
// Same re-aim as the primary edit: a stream may have grown the transcript during the confirm.
|
||||
const messages = forced.confirmDeepTruncate ? readMessages() : planning
|
||||
const plan = messages === planning ? forced : revalidateEditPlan(forced, messages, edited)
|
||||
const plan = messages === planning ? forced : revalidateEditPlan(forced, planning, messages, edited)
|
||||
|
||||
if (!plan) {
|
||||
// The edited turn moved while the confirm was open: nothing was sent.
|
||||
@@ -772,15 +774,16 @@ export function useSessionTileActions({ requestGateway, runtimeId, scope, stored
|
||||
} else if ((await confirmDeep()) && runtimeIdRef.current === sessionId) {
|
||||
// Re-run from the top: the session is re-read and re-validated after this wait. The answer
|
||||
// belongs to this runtime: a rebind during the dialog drops it.
|
||||
await editMessageRef.current(edited, planning)
|
||||
await runEditRef.current(edited, planning)
|
||||
}
|
||||
}
|
||||
},
|
||||
[applySurvivorRowIds, copy.editFailed, readMessages, readState, submitRewind, t, update]
|
||||
)
|
||||
|
||||
const editMessageRef = useRef(editMessage)
|
||||
editMessageRef.current = editMessage
|
||||
const runEditRef = useRef(runEdit)
|
||||
runEditRef.current = runEdit
|
||||
const editMessage = useCallback((edited: AppendMessage) => runEdit(edited), [runEdit])
|
||||
|
||||
// Branch-visibility sync (assistant-ui hides non-active branches).
|
||||
const handleThreadMessagesChange = useCallback(
|
||||
|
||||
@@ -1002,7 +1002,8 @@ export function usePromptActions({
|
||||
[activeSessionIdRef, requestGateway, selectedStoredSessionIdRef]
|
||||
)
|
||||
|
||||
const reloadFromMessage = useCallback(
|
||||
// answeredFor is internal (the 4033 re-run); assistant-ui calls onReload(parentId, config).
|
||||
const runReload = useCallback(
|
||||
async (parentId: string | null, answeredFor?: ChatMessage[]): Promise<void> => {
|
||||
// Ref, not the closure-captured prop — a truncating resubmit aimed at a
|
||||
// stale session deletes the wrong transcript.
|
||||
@@ -1062,15 +1063,16 @@ export function usePromptActions({
|
||||
} else if ((await confirmDeep()) && activeSessionIdRef.current === sessionId) {
|
||||
// Re-run from the top: the session is re-read and re-validated after this wait. The answer
|
||||
// belongs to this session: a switch during the dialog drops it.
|
||||
await reloadFromMessageRef.current(parentId, messages)
|
||||
await runReloadRef.current(parentId, messages)
|
||||
}
|
||||
}
|
||||
},
|
||||
[activeSessionIdRef, applySurvivorRowIds, copy.regenerateFailed, submitRewindPrompt, t, updateSessionState]
|
||||
)
|
||||
|
||||
const reloadFromMessageRef = useRef(reloadFromMessage)
|
||||
reloadFromMessageRef.current = reloadFromMessage
|
||||
const runReloadRef = useRef(runReload)
|
||||
runReloadRef.current = runReload
|
||||
const reloadFromMessage = useCallback((parentId: string | null) => runReload(parentId), [runReload])
|
||||
|
||||
// Cursor-style "restore checkpoint": rewind the conversation to a past user
|
||||
// prompt and run it again from there. Reuses the edit composer's rewind
|
||||
@@ -1204,7 +1206,7 @@ export function usePromptActions({
|
||||
]
|
||||
)
|
||||
|
||||
const editMessage = useCallback(
|
||||
const runEdit = useCallback(
|
||||
async (edited: AppendMessage, answeredFor?: ChatMessage[]): Promise<void> => {
|
||||
// Ref, not the closure-captured prop — an edit rewinds and resubmits, so
|
||||
// a stale target rewrites the wrong session's history.
|
||||
@@ -1230,7 +1232,7 @@ export function usePromptActions({
|
||||
// re-aimed at the current transcript (a stream may have grown it) and dropped, with a notice,
|
||||
// only if the edited turn itself moved.
|
||||
const messages = forced.confirmDeepTruncate ? currentMessages(sessionId) : planning
|
||||
const plan = messages === planning ? forced : revalidateEditPlan(forced, messages, edited)
|
||||
const plan = messages === planning ? forced : revalidateEditPlan(forced, planning, messages, edited)
|
||||
|
||||
if (!plan) {
|
||||
// The edited turn moved while the confirm was open: nothing was sent.
|
||||
@@ -1357,7 +1359,7 @@ export function usePromptActions({
|
||||
} else if ((await confirmDeep()) && activeSessionIdRef.current === sessionId) {
|
||||
// Re-run from the top: the session is re-read and re-validated after this wait. The answer
|
||||
// belongs to this session: a switch during the dialog drops it.
|
||||
await editMessageRef.current(edited, planning)
|
||||
await runEditRef.current(edited, planning)
|
||||
}
|
||||
}
|
||||
},
|
||||
@@ -1375,8 +1377,9 @@ export function usePromptActions({
|
||||
]
|
||||
)
|
||||
|
||||
const editMessageRef = useRef(editMessage)
|
||||
editMessageRef.current = editMessage
|
||||
const runEditRef = useRef(runEdit)
|
||||
runEditRef.current = runEdit
|
||||
const editMessage = useCallback((edited: AppendMessage) => runEdit(edited), [runEdit])
|
||||
|
||||
const handleThreadMessagesChange = useCallback(
|
||||
(nextMessages: readonly ThreadMessage[]) => {
|
||||
|
||||
@@ -16,6 +16,7 @@ import {
|
||||
planRestore,
|
||||
rebindSurvivorRowIds,
|
||||
resolveDurableRowId,
|
||||
revalidateEditPlan,
|
||||
runRewindSubmit,
|
||||
survivorRowIdsFrom,
|
||||
truncateSubmitParams
|
||||
@@ -808,3 +809,27 @@ describe('edit and server-refusal confirms (#133716 review)', () => {
|
||||
expect(asked).toBe(1)
|
||||
})
|
||||
})
|
||||
|
||||
describe('revalidateEditPlan (#133716)', () => {
|
||||
const planned = [
|
||||
row('u1', 'user', 'old prompt', { rowId: 11 }),
|
||||
row('a1', 'assistant', 'old reply'),
|
||||
row('u2', 'user', 'latest prompt', { rowId: 13 }),
|
||||
row('a2', 'assistant', 'latest reply')
|
||||
]
|
||||
|
||||
const edit = { role: 'user', sourceId: 'u1', parentId: null, content: [{ type: 'text', text: 'edited' }] } as never
|
||||
|
||||
it('follows a transcript a stream grew; refuses one that gained a later user turn', () => {
|
||||
const plan = { ...planEdit(planned, edit)!, confirmDeepTruncate: true }
|
||||
const streamed = [...planned, row('a2b', 'assistant', 'more')]
|
||||
|
||||
expect(revalidateEditPlan(plan, planned, streamed, edit)).toMatchObject({
|
||||
confirmDeepTruncate: true,
|
||||
truncateRowId: 11
|
||||
})
|
||||
expect(
|
||||
revalidateEditPlan(plan, planned, [...streamed, row('u3', 'user', 'unseen', { rowId: 15 })], edit)
|
||||
).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -747,16 +747,22 @@ export async function planConfirmedEdit(
|
||||
*/
|
||||
export function revalidateEditPlan<P extends EditPlan>(
|
||||
plan: P,
|
||||
planned: ChatMessage[],
|
||||
current: ChatMessage[],
|
||||
edited: AppendMessage
|
||||
): P | null {
|
||||
const fresh = planEdit(current, edited)
|
||||
|
||||
const laterUsers = (messages: ChatMessage[], index: number) =>
|
||||
messages.filter((m, i) => i > index && isVisibleUserMessage(m)).length
|
||||
|
||||
// A new later user turn landed during the confirm: the user never answered for archiving it.
|
||||
if (
|
||||
!fresh ||
|
||||
fresh.truncateMessageId !== plan.truncateMessageId ||
|
||||
fresh.truncateRowId !== plan.truncateRowId ||
|
||||
fresh.truncateOrdinal !== plan.truncateOrdinal
|
||||
fresh.truncateOrdinal !== plan.truncateOrdinal ||
|
||||
laterUsers(current, fresh.sourceIndex) !== laterUsers(planned, plan.sourceIndex)
|
||||
) {
|
||||
return null
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user