mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix(ai-chat): stop the session chat claiming a model cannot think when we cannot read it
The session chat renders the same model button, so it carried the same flaw the flow chat just lost: on `customai` / `groq` / `togetherai` it drew "Not supported by this model" and `selectModel` dropped the pinned effort, with no slider left to put one back. Both now read `known`. `carriedReasoning` is where that belongs — asked what survives a model change, the answer for a model we have no rules for is "what you had". The flow chat keeps its own guard on top, since writing the value back would still touch storage. `turnFailed` moves to turnTranscript.ts, whose subject is exactly this — how a turn reads from its rows — and which can be imported by a test without dragging in Monaco. Five cases pin it: recovered tool, terminal failure, still streaming, the window's end, and a turn with nothing in it yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QN7VboDEm9HAB1t4sMxMdE
This commit is contained in:
co-authored by
Claude Opus 5
parent
3678c8b51f
commit
daa4750a76
@@ -247,16 +247,20 @@
|
||||
}))
|
||||
}
|
||||
],
|
||||
reasoning: {
|
||||
provider: providerModel.provider as AIProvider,
|
||||
model: providerModel.model,
|
||||
value: providerModel.reasoning,
|
||||
offToken: REASONING_OFF,
|
||||
// The copilot fills an unset effort in before it calls the provider, so unset
|
||||
// really does run at the default level and the button may name it.
|
||||
sendsDefaultWhenUnset: true,
|
||||
onSelect: selectReasoning
|
||||
},
|
||||
// Silent where the registry has no rules for the provider: claiming the model cannot
|
||||
// think would be a guess, and the row saying so is the only thing that would render.
|
||||
reasoning: getReasoningCapability(providerModel.provider, providerModel.model).known
|
||||
? {
|
||||
provider: providerModel.provider as AIProvider,
|
||||
model: providerModel.model,
|
||||
value: providerModel.reasoning,
|
||||
offToken: REASONING_OFF,
|
||||
// The copilot fills an unset effort in before it calls the provider, so unset
|
||||
// really does run at the default level and the button may name it.
|
||||
sendsDefaultWhenUnset: true,
|
||||
onSelect: selectReasoning
|
||||
}
|
||||
: undefined,
|
||||
// A reading preference rather than a model parameter: it applies to every chat in
|
||||
// this browser, including thinking already in the transcript. No close(): flipping
|
||||
// it should not dismiss the menu.
|
||||
|
||||
@@ -84,13 +84,18 @@ export const REASONING_PROVIDER_DEFAULT = 'default'
|
||||
* level. Dropped rather than carried because a model that cannot think at that level either
|
||||
* rejects the request or quietly runs at another one, and the button would name a level the
|
||||
* run never used. Off survives only onto a model that can truly disable.
|
||||
*
|
||||
* A model the registry has no rules for keeps whatever it had: dropping on `supported:
|
||||
* false` would discard a real setting on the strength of never having heard of the
|
||||
* provider, and nothing would draw a control to put it back.
|
||||
*/
|
||||
export function carriedReasoning(
|
||||
current: string | undefined,
|
||||
offToken: string | undefined,
|
||||
capability: { levels: string[]; canDisable: boolean }
|
||||
capability: { levels: string[]; canDisable: boolean; known: boolean }
|
||||
): string | undefined {
|
||||
if (current === undefined || current === '') return undefined
|
||||
if (!capability.known) return current
|
||||
if (offToken !== undefined && current === offToken) {
|
||||
return capability.canDisable ? current : undefined
|
||||
}
|
||||
|
||||
@@ -16,6 +16,7 @@ import type { AttachedTextFile } from '$lib/components/copilot/chat/textFileUtil
|
||||
import { HelpersService } from '$lib/gen'
|
||||
import { sendUserToast } from '$lib/toast'
|
||||
import { randomUUID } from '$lib/utils/uuid'
|
||||
import { turnFailed } from './turnTranscript'
|
||||
import {
|
||||
attachmentsToMessageInputs,
|
||||
MessageInputsStore,
|
||||
@@ -134,27 +135,6 @@ function toDisplayMessage(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the turn a user message started ended without an answer.
|
||||
*
|
||||
* Read from the turn's last row and no other. A tool that fails mid-turn is handed back to
|
||||
* the agent, which routinely recovers and answers, so an unsuccessful tool row says nothing
|
||||
* about the turn — and this drives the Retry button, which in the copilot means "the request
|
||||
* never went through" rather than "something inside it went wrong". Offering it for a turn
|
||||
* that answered would invite running the whole flow a second time, side effects and all.
|
||||
*/
|
||||
function turnFailed(messages: ChatMessage[], userIndex: number): boolean {
|
||||
let last: ChatMessage | undefined
|
||||
for (let i = userIndex + 1; i < messages.length; i++) {
|
||||
const message = messages[i]
|
||||
if (message.message_type === 'user') break
|
||||
// Still going, so the turn has no outcome to report yet.
|
||||
if (message.streaming || message.loading) return false
|
||||
last = message
|
||||
}
|
||||
return last?.success === false
|
||||
}
|
||||
|
||||
/**
|
||||
* Renders a flow run's conversation through the AI session chat components. The
|
||||
* turn itself is a flow job, so everything the copilot's own loop owns — context
|
||||
|
||||
@@ -1,5 +1,12 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { appendRevealed, applyStreamEvent, emptyTurnState, type TurnStep } from './turnTranscript'
|
||||
import {
|
||||
appendRevealed,
|
||||
applyStreamEvent,
|
||||
emptyTurnState,
|
||||
turnFailed,
|
||||
type TurnStep
|
||||
} from './turnTranscript'
|
||||
import type { ChatMessage } from './FlowChatManager.svelte'
|
||||
import type { StreamEvent } from '$lib/components/chat/utils'
|
||||
|
||||
function start(): TurnStep {
|
||||
@@ -75,3 +82,55 @@ describe('turn transcript', () => {
|
||||
expect(step.rows[0].streaming).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
/**
|
||||
* Gates the Retry button, which on a flow re-runs the whole thing — side effects included —
|
||||
* so it has to mean "this turn produced no answer", not "something inside it went wrong".
|
||||
*/
|
||||
describe('turnFailed', () => {
|
||||
const row = (over: Partial<ChatMessage>): ChatMessage =>
|
||||
({ id: 'x', message_type: 'assistant', content: '', ...over }) as ChatMessage
|
||||
|
||||
it('is false when a tool failed but the agent went on to answer', () => {
|
||||
const messages = [
|
||||
row({ message_type: 'user' }),
|
||||
row({ message_type: 'tool', success: false }),
|
||||
row({ message_type: 'assistant', success: true })
|
||||
]
|
||||
expect(turnFailed(messages, 0)).toBe(false)
|
||||
})
|
||||
|
||||
it('is true when the turn ends on a failure', () => {
|
||||
const messages = [
|
||||
row({ message_type: 'user' }),
|
||||
row({ message_type: 'tool', success: true }),
|
||||
row({ message_type: 'assistant', success: false })
|
||||
]
|
||||
expect(turnFailed(messages, 0)).toBe(true)
|
||||
})
|
||||
|
||||
it('reports nothing while the turn is still running', () => {
|
||||
const messages = [
|
||||
row({ message_type: 'user' }),
|
||||
row({ message_type: 'tool', success: false }),
|
||||
row({ message_type: 'assistant', streaming: true })
|
||||
]
|
||||
expect(turnFailed(messages, 0)).toBe(false)
|
||||
})
|
||||
|
||||
// The window stops at the next user message, so a later turn's failure is not this one's.
|
||||
it('does not read past the next user message', () => {
|
||||
const messages = [
|
||||
row({ message_type: 'user' }),
|
||||
row({ message_type: 'assistant', success: true }),
|
||||
row({ message_type: 'user' }),
|
||||
row({ message_type: 'assistant', success: false })
|
||||
]
|
||||
expect(turnFailed(messages, 0)).toBe(false)
|
||||
expect(turnFailed(messages, 2)).toBe(true)
|
||||
})
|
||||
|
||||
it('is false for a turn that has produced nothing yet', () => {
|
||||
expect(turnFailed([row({ message_type: 'user' })], 0)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -161,3 +161,24 @@ export function applyStreamEvent(
|
||||
return step
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the turn a user message started ended without an answer.
|
||||
*
|
||||
* Read from the turn's last row and no other. A tool that fails mid-turn is handed back to
|
||||
* the agent, which routinely recovers and answers, so an unsuccessful tool row says nothing
|
||||
* about the turn — and this drives the Retry button, which in the copilot means "the request
|
||||
* never went through" rather than "something inside it went wrong". Offering it for a turn
|
||||
* that answered would invite running the whole flow a second time, side effects and all.
|
||||
*/
|
||||
export function turnFailed(messages: ChatMessage[], userIndex: number): boolean {
|
||||
let last: ChatMessage | undefined
|
||||
for (let i = userIndex + 1; i < messages.length; i++) {
|
||||
const message = messages[i]
|
||||
if (message.message_type === 'user') break
|
||||
// Still going, so the turn has no outcome to report yet.
|
||||
if (message.streaming || message.loading) return false
|
||||
last = message
|
||||
}
|
||||
return last?.success === false
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user