mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-07 16:03:21 +00:00
fix(ai-chat): keep the composer usable while a question is pending (#10816)
* fix(ai-chat): keep the composer usable while a question is pending Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D2hyAfRT2aFF7uswsdTodL * fix(ai-chat): keep a typed answer when the question's resolver is gone Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D2hyAfRT2aFF7uswsdTodL * fix(ai-chat): only advertise the answer affordance on a live question Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D2hyAfRT2aFF7uswsdTodL * style: trim the pending-question rationale comments to the 4-line cap Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D2hyAfRT2aFF7uswsdTodL --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
29c311ab31
commit
25a3e6ea7a
@@ -26,7 +26,7 @@
|
||||
import { fade } from 'svelte/transition'
|
||||
import Popover from '$lib/components/meltComponents/Popover.svelte'
|
||||
import DropdownV2 from '$lib/components/DropdownV2.svelte'
|
||||
import { pendingUserAction, type DisplayMessage } from './shared'
|
||||
import { pendingUserAction, pendingUserActionDetail, type DisplayMessage } from './shared'
|
||||
import { PLAN_MODE_TEXT_COLOR, PLAN_MODE_TRIGGER_CLASS } from './planMode'
|
||||
import { PLAN_MODE_MESSAGES } from './planModeMessages'
|
||||
import type { ContextElement } from './context'
|
||||
@@ -514,10 +514,16 @@
|
||||
// act on the tool above.
|
||||
const waitingForUserAction = $derived(aiChatManager.loading && !!pendingUserAction(messages))
|
||||
|
||||
// While the AI is waiting on an answer to an askUserQuestion, the only valid
|
||||
// input is one of the choices (or the custom answer) in the question card —
|
||||
// so disable the main chat input until the question is answered or canceled.
|
||||
const hasActiveUserQuestion = $derived(pendingUserAction(messages) === 'question')
|
||||
// Gated on `loading` because a card restored from history still looks parked:
|
||||
// its resolver left with the old page, so the composer must not advertise an
|
||||
// answer it cannot deliver.
|
||||
const pendingQuestionToolCallId = $derived.by(() => {
|
||||
if (!aiChatManager.loading) {
|
||||
return undefined
|
||||
}
|
||||
const pending = pendingUserActionDetail(messages)
|
||||
return pending?.action === 'question' ? pending.toolCallId : undefined
|
||||
})
|
||||
|
||||
// Get app context for display when in APP mode
|
||||
const appContext = $derived.by((): SelectedContext | undefined => {
|
||||
@@ -799,7 +805,8 @@ the panel, or the Escape-to-stop focus check would wrongly reject them. -->
|
||||
{initialInstructions}
|
||||
{onDraftChange}
|
||||
showContext={aiChatManager.mode !== AIMode.GLOBAL}
|
||||
disabled={disabled || hasActiveUserQuestion}
|
||||
{disabled}
|
||||
{pendingQuestionToolCallId}
|
||||
isFirstMessage={messages.length === 0}
|
||||
/>
|
||||
<div
|
||||
|
||||
@@ -76,6 +76,10 @@
|
||||
// `aiChatManager.instructions` only carries programmatic prompts). Used by
|
||||
// sessions to persist the typed-but-unsent prompt with the session draft.
|
||||
onDraftChange?: (text: string) => void
|
||||
// tool_call_id of the askUserQuestion the turn is parked on, when it is. A
|
||||
// plain-text draft sent from here answers it instead of queueing behind a
|
||||
// turn that only the answer can resume.
|
||||
pendingQuestionToolCallId?: string
|
||||
}
|
||||
|
||||
let {
|
||||
@@ -98,7 +102,8 @@
|
||||
onKeyDown = undefined,
|
||||
loading,
|
||||
onCancel,
|
||||
onDraftChange = undefined
|
||||
onDraftChange = undefined,
|
||||
pendingQuestionToolCallId = undefined
|
||||
}: Props = $props()
|
||||
|
||||
// GLOBAL-mode suggestion pool. We pick one at mount-time so each new
|
||||
@@ -124,6 +129,10 @@
|
||||
|
||||
// Generate mode-specific placeholder
|
||||
const modePlaceholder = $derived.by(() => {
|
||||
if (pendingQuestionToolCallId !== undefined) {
|
||||
return 'Answer the question above'
|
||||
}
|
||||
|
||||
if (!isFirstMessage) {
|
||||
return 'Ask followup'
|
||||
}
|
||||
@@ -182,6 +191,17 @@
|
||||
onDraftChange?.(text)
|
||||
})
|
||||
})
|
||||
|
||||
// A parked askUserQuestion is a request for input, so a plain-text draft sent
|
||||
// from the composer answers it rather than being queued behind a turn that can
|
||||
// only resume once the question is answered. Attachments can't ride an answer,
|
||||
// so a draft carrying them falls through to the queue and flushes on resume.
|
||||
const questionAnsweredBySend = $derived(
|
||||
editingMessageIndex === null && draft.text.trim() !== '' && !draft.hasAttachments
|
||||
? pendingQuestionToolCallId
|
||||
: undefined
|
||||
)
|
||||
|
||||
// Images being decoded right now. Holds off sending so a message can never go
|
||||
// out without an attachment the user already dropped, and reserves cap slots
|
||||
// against a concurrent drop.
|
||||
@@ -674,6 +694,20 @@
|
||||
if (pendingImages > 0 || pendingFiles > 0 || ingestionHolds > 0) {
|
||||
return
|
||||
}
|
||||
// Read before `take()` empties the draft the id derives from, and only take
|
||||
// once the answer is delivered — an undelivered one would leave the user
|
||||
// with neither their text nor a resumed turn.
|
||||
const answeredQuestionId = questionAnsweredBySend
|
||||
if (
|
||||
answeredQuestionId &&
|
||||
aiChatManager.handleUserQuestionAnswer(answeredQuestionId, [
|
||||
expanded(chatDraft(draft.text.trim(), draft.pastes))
|
||||
])
|
||||
) {
|
||||
draft.take()
|
||||
contextTextareaComponent?.clearForSend()
|
||||
return
|
||||
}
|
||||
if (aiChatManager.loading) {
|
||||
// Queue the message instead of silently discarding it — it is
|
||||
// auto-sent when the streaming turn completes successfully.
|
||||
@@ -940,7 +974,10 @@
|
||||
</script>
|
||||
|
||||
{#snippet sendStopButton()}
|
||||
{@const isLoading = loading ?? aiChatManager.loading}
|
||||
<!-- The turn stays `loading` while parked on a question, but a drafted answer
|
||||
is what the button should ship then — otherwise the only pointer action on
|
||||
a typed answer would be Stop. Anything else keeps Stop. -->
|
||||
{@const isLoading = (loading ?? aiChatManager.loading) && !questionAnsweredBySend}
|
||||
{@const emptyDraft = draft.isEmpty}
|
||||
<!-- A text-free GLOBAL draft with context chips is a valid turn (Enter
|
||||
already sends it), so the button stays enabled there for pointer/touch
|
||||
|
||||
@@ -1685,10 +1685,13 @@ export class AIChatManager {
|
||||
})
|
||||
}
|
||||
|
||||
handleUserQuestionAnswer = (toolId: string, choices: string[]) => {
|
||||
/** Returns whether the answer was delivered: a card restored from history
|
||||
* still looks parked but its resolver is gone with the old page, so callers
|
||||
* holding the only copy of the answer must not discard it on a false. */
|
||||
handleUserQuestionAnswer = (toolId: string, choices: string[]): boolean => {
|
||||
const callback = this.userQuestionCallbacks.get(toolId)
|
||||
if (!callback) {
|
||||
return
|
||||
return false
|
||||
}
|
||||
|
||||
// Display-only readback for the collapsed tool-header: a compact comma list.
|
||||
@@ -1713,6 +1716,7 @@ export class AIChatManager {
|
||||
|
||||
callback(choices)
|
||||
this.userQuestionCallbacks.delete(toolId)
|
||||
return true
|
||||
}
|
||||
|
||||
setAiChatInput(aiChatInput: AIChatInput | null) {
|
||||
|
||||
@@ -701,6 +701,33 @@ describe('AIChatManager persisted autonomy default', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('AIChatManager user questions', () => {
|
||||
// The composer holds the only copy of a typed answer and clears it on a true,
|
||||
// so a card whose resolver is gone (restored from history, its promise left
|
||||
// with the old page) must report the answer as undelivered.
|
||||
it('reports whether the answer reached a waiting resolver', () => {
|
||||
const manager = new AIChatManager()
|
||||
manager.displayMessages = [
|
||||
{
|
||||
role: 'tool',
|
||||
tool_call_id: 'call_ask',
|
||||
content: 'asking',
|
||||
isLoading: true,
|
||||
userQuestion: { question: 'Pick one', choices: ['a', 'b'] }
|
||||
}
|
||||
]
|
||||
|
||||
expect(manager.handleUserQuestionAnswer('call_ask', ['a'])).toBe(false)
|
||||
|
||||
const answered = manager.requestUserQuestion('call_ask', {
|
||||
question: 'Pick one',
|
||||
choices: ['a', 'b']
|
||||
})
|
||||
expect(manager.handleUserQuestionAnswer('call_ask', ['a'])).toBe(true)
|
||||
return expect(answered).resolves.toEqual(['a'])
|
||||
})
|
||||
})
|
||||
|
||||
describe('AIChatManager queued messages', () => {
|
||||
const model = { provider: 'openai', model: 'gpt-4o' }
|
||||
|
||||
|
||||
@@ -49,6 +49,10 @@ export class MessageDraft {
|
||||
)
|
||||
}
|
||||
|
||||
get hasAttachments(): boolean {
|
||||
return this.images.length > 0 || this.files.length > 0
|
||||
}
|
||||
|
||||
/** Files joining a draft always fold (dedupe by source identity, courtesy
|
||||
* rename) and respect the slot cap. `byteBudget`, when given, admits the
|
||||
* folded entries by their decoded size — the fold must run first because
|
||||
|
||||
@@ -1319,6 +1319,20 @@ describe('pendingUserAction', () => {
|
||||
const userMessage: DisplayMessage = { role: 'user', index: 0, content: 'go on' }
|
||||
expect(pendingUserAction([question, userMessage, toolMessage()])).toBe(undefined)
|
||||
})
|
||||
|
||||
// The composer answers the parked question by id, so the scan must name the
|
||||
// blocked card and not one of the queued ones sharing the turn.
|
||||
it('names the blocked card so a caller can resolve it', async () => {
|
||||
const { pendingUserActionDetail } = await import('./shared')
|
||||
const blocked = toolMessage({
|
||||
tool_call_id: 'call_ask',
|
||||
userQuestion: { question: 'Pick one', choices: ['a'] }
|
||||
})
|
||||
expect(pendingUserActionDetail([blocked, toolMessage({ tool_call_id: 'call_next' })])).toEqual({
|
||||
action: 'question',
|
||||
toolCallId: 'call_ask'
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('pollJobCompletion detach', () => {
|
||||
|
||||
@@ -657,8 +657,8 @@ export type DisplayMessage =
|
||||
|
||||
// A tool message whose askUserQuestion is still awaiting an answer: the AI loop
|
||||
// is paused on the user. Drives the question card's interactivity, the
|
||||
// "waiting for user" indicator, and disabling the main chat input — keep those
|
||||
// in sync by going through this single predicate.
|
||||
// "waiting for user" indicator, and routing a composer send to the answer —
|
||||
// keep those in sync by going through this single predicate.
|
||||
export function isActiveUserQuestion(message: DisplayMessage | undefined): boolean {
|
||||
return Boolean(
|
||||
message &&
|
||||
@@ -676,17 +676,27 @@ export function isActiveUserQuestion(message: DisplayMessage | undefined): boole
|
||||
// rendering progress must ask here first or it reports "the AI is working".
|
||||
export type PendingUserAction = 'question' | 'confirmation'
|
||||
|
||||
export function pendingUserAction(messages: DisplayMessage[]): PendingUserAction | undefined {
|
||||
return pendingUserActionDetail(messages)?.action
|
||||
}
|
||||
|
||||
// Scans back to the turn boundary, not just the last message: a turn's cards are
|
||||
// created up front and run one at a time, and text between two tool calls pushes
|
||||
// an assistant card between them, so the blocked card is rarely last. Only cards
|
||||
// of a live turn can match — every resolution path clears `isLoading`.
|
||||
export function pendingUserAction(messages: DisplayMessage[]): PendingUserAction | undefined {
|
||||
export function pendingUserActionDetail(
|
||||
messages: DisplayMessage[]
|
||||
): { action: PendingUserAction; toolCallId: string } | undefined {
|
||||
for (let i = messages.length - 1; i >= 0; i--) {
|
||||
const message = messages[i]
|
||||
if (message.role === 'user') break
|
||||
if (message.role !== 'tool') continue
|
||||
if (isActiveUserQuestion(message)) return 'question'
|
||||
if (message.needsConfirmation && message.isLoading) return 'confirmation'
|
||||
if (isActiveUserQuestion(message)) {
|
||||
return { action: 'question', toolCallId: message.tool_call_id }
|
||||
}
|
||||
if (message.needsConfirmation && message.isLoading) {
|
||||
return { action: 'confirmation', toolCallId: message.tool_call_id }
|
||||
}
|
||||
}
|
||||
return undefined
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user