mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix(ai-chat): tell a failed turn from a recovered one, and a model that cannot think from one we cannot read
- `turnFailed` marked a turn failed on any unsuccessful tool row, but a failing tool is handed back to the agent, which routinely recovers and answers. That put a Retry on turns that succeeded — and retrying re-runs the whole flow, side effects included. It now reads the turn's terminal row, and reports nothing while the turn is still streaming. `error` on a user row means what it means in the copilot: the request never produced an answer. - The reasoning registry answered `supported: false` for every model on a provider family it has no rules for, so `customai` — which fronts any OpenAI-compatible endpoint — got "Not supported by this model" stated as a fact, its effort input hidden from the modal, and its stored value cleared to `''` on every model pick, shadowing the author's schema default for good. `ReasoningCapability` now carries `known`, and the composer neither claims anything nor touches the value where it cannot read the model. - `flow-base.md`: a static provider field is not "shown read-only"; the composer draws no control for it at all. 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
b40219652f
commit
3678c8b51f
Generated
+3
-2
@@ -5265,8 +5265,9 @@ input, read by the agent.
|
||||
|
||||
**Wire the provider field by field, or the chat cannot change its model.** Each \`provider\` field fed
|
||||
by a flow input becomes a control in the composer — a provider picker, a model list, a thinking
|
||||
slider — while a field left static is fixed and shown read-only. \`user_attachments\` works the same
|
||||
way: point it at an s3-object input and the composer gets a paperclip.
|
||||
slider — while a field left static is fixed, and the composer draws no control for it.
|
||||
\`user_attachments\` works the same way: point it at an s3-object input and the composer gets a
|
||||
paperclip.
|
||||
|
||||
\`\`\`json
|
||||
{
|
||||
|
||||
@@ -27,7 +27,7 @@
|
||||
let capability = $derived(
|
||||
provider && model
|
||||
? getReasoningCapability(provider, model)
|
||||
: { supported: false, levels: [], canDisable: false }
|
||||
: { supported: false, levels: [], canDisable: false, known: false }
|
||||
)
|
||||
|
||||
// The token that turns reasoning off on a model that reasons by default
|
||||
|
||||
@@ -31,7 +31,7 @@
|
||||
const capability = $derived(
|
||||
reasoning
|
||||
? getReasoningCapability(reasoning.provider, reasoning.model)
|
||||
: { supported: false, levels: [] as string[], canDisable: false }
|
||||
: { supported: false, levels: [] as string[], canDisable: false, known: false }
|
||||
)
|
||||
// Effective effort accounts for the default-on level on capable models.
|
||||
const effective = $derived(
|
||||
|
||||
@@ -77,9 +77,9 @@ describe('supportsReasoning (static registry)', () => {
|
||||
}
|
||||
// Bedrock translates the same sentinel on its Converse path, but only for
|
||||
// Opus 5 — AWS documents Bedrock's Sonnet 5 as always thinking.
|
||||
expect(
|
||||
getReasoningCapability('aws_bedrock', 'global.anthropic.claude-opus-5').canDisable
|
||||
).toBe(true)
|
||||
expect(getReasoningCapability('aws_bedrock', 'global.anthropic.claude-opus-5').canDisable).toBe(
|
||||
true
|
||||
)
|
||||
expect(
|
||||
resolveRequestReasoning({
|
||||
provider: 'aws_bedrock',
|
||||
@@ -189,13 +189,29 @@ describe('supportsReasoning (static registry)', () => {
|
||||
expect(supportsReasoning('mistral', 'mistral-medium-3.5')).toBe(true)
|
||||
expect(getReasoningCapability('mistral', 'mistral-medium-3-5').canDisable).toBe(true)
|
||||
})
|
||||
it('returns no levels for providers without a registry entry', () => {
|
||||
it('returns no levels for a model its provider family has no entry for', () => {
|
||||
// The family is known, so the `false` is an answer: codestral does not reason.
|
||||
expect(getReasoningCapability('mistral', 'codestral-latest')).toEqual({
|
||||
supported: false,
|
||||
levels: [],
|
||||
canDisable: false
|
||||
canDisable: false,
|
||||
known: true
|
||||
})
|
||||
})
|
||||
|
||||
// `customai` fronts any OpenAI-compatible endpoint, so `supported: false` there is an
|
||||
// absence of rules rather than a fact about the model. A caller that shows the reader
|
||||
// "this model cannot think" has to tell the two apart.
|
||||
it('admits when it has no rules for the provider at all', () => {
|
||||
expect(getReasoningCapability('customai', 'deepseek-r1')).toEqual({
|
||||
supported: false,
|
||||
levels: [],
|
||||
canDisable: false,
|
||||
known: false
|
||||
})
|
||||
expect(getReasoningCapability('openai', 'gpt-4o').known).toBe(true)
|
||||
expect(getReasoningCapability('anthropic', 'claude-sonnet-5').known).toBe(true)
|
||||
})
|
||||
it('only offers off where the model can truly disable thinking', () => {
|
||||
// Gemini Pro enforces a thinking floor — no off option.
|
||||
expect(getReasoningCapability('googleai', 'gemini-2.5-pro').canDisable).toBe(false)
|
||||
|
||||
@@ -231,14 +231,35 @@ export type ReasoningCapability = {
|
||||
* level, making the switch a lie.
|
||||
*/
|
||||
canDisable: boolean
|
||||
/**
|
||||
* Whether `supported` is an answer or an absence of one. The registry has rules per
|
||||
* provider family and falls through to `false` for the rest — `customai` above all,
|
||||
* which fronts any OpenAI-compatible endpoint and may well serve a thinking model. A
|
||||
* caller that presents `supported: false` as a fact must check this first, or it tells
|
||||
* the reader a model cannot think when all we know is that we have never heard of it.
|
||||
*/
|
||||
known: boolean
|
||||
}
|
||||
|
||||
/** Provider families the registry has real rules for; everything else is a shrug. */
|
||||
const KNOWN_REASONING_FAMILIES: ReadonlySet<string> = new Set([
|
||||
'anthropic',
|
||||
'aws_bedrock',
|
||||
'openai',
|
||||
'azure_openai',
|
||||
'openrouter',
|
||||
'googleai',
|
||||
'deepseek',
|
||||
'mistral'
|
||||
])
|
||||
|
||||
/** Resolve the reasoning capability of a model from the static registry. */
|
||||
export function getReasoningCapability(provider: AIProvider, model: string): ReasoningCapability {
|
||||
const bareModel = stripLegacyThinkingSuffix(model)
|
||||
const known = KNOWN_REASONING_FAMILIES.has(reasoningProviderFamily(provider, bareModel))
|
||||
const supported = supportsReasoningStatic(provider, bareModel)
|
||||
if (!supported) {
|
||||
return { supported: false, levels: [], canDisable: false }
|
||||
return { supported: false, levels: [], canDisable: false, known }
|
||||
}
|
||||
const family = reasoningProviderFamily(provider, bareModel)
|
||||
const levels =
|
||||
@@ -251,7 +272,7 @@ export function getReasoningCapability(provider: AIProvider, model: string): Rea
|
||||
: family === 'openrouter'
|
||||
? openrouterReasoningLevels(bareModel)
|
||||
: (PROVIDER_REASONING_LEVELS[family] ?? ['low', 'medium', 'high'])
|
||||
return { supported, levels, canDisable: canDisableReasoning(provider, bareModel) }
|
||||
return { supported, levels, canDisable: canDisableReasoning(provider, bareModel), known }
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -362,15 +383,11 @@ export function explicitOffToken(provider: AIProvider, model: string): Reasoning
|
||||
// real off there and stays the wire form. Only the 5 family, which
|
||||
// thinks when the field is absent, needs the explicit disable —
|
||||
// Fable and Mythos reject it outright and get no off token at all.
|
||||
return /claude-(opus|sonnet)-5/.test(model.toLowerCase())
|
||||
? ANTHROPIC_OFF_SENTINEL
|
||||
: undefined
|
||||
return /claude-(opus|sonnet)-5/.test(model.toLowerCase()) ? ANTHROPIC_OFF_SENTINEL : undefined
|
||||
case 'aws_bedrock':
|
||||
// Bedrock's Sonnet 5 cannot be disabled at all, so only Opus 5 gets
|
||||
// the sentinel; the rest keep omission.
|
||||
return model.toLowerCase().includes('claude-opus-5')
|
||||
? ANTHROPIC_OFF_SENTINEL
|
||||
: undefined
|
||||
return model.toLowerCase().includes('claude-opus-5') ? ANTHROPIC_OFF_SENTINEL : undefined
|
||||
case 'googleai':
|
||||
// Gemini 2.5/3 think by default (dynamic budget / level). The backend
|
||||
// proxy maps 'none' to off on Flash, or the floor on Pro (only
|
||||
|
||||
@@ -8,7 +8,8 @@
|
||||
import Modal from '$lib/components/common/modal/Modal.svelte'
|
||||
import SchemaForm from '$lib/components/SchemaForm.svelte'
|
||||
import { type DynamicInput } from '$lib/utils'
|
||||
import { type FlowModule } from '$lib/gen'
|
||||
import { type AIProvider, type FlowModule } from '$lib/gen'
|
||||
import { getReasoningCapability } from '$lib/components/copilot/reasoningRegistry'
|
||||
import { useWorkspaceStorageConfigured } from '$lib/components/inputTransformEnv.svelte'
|
||||
import { workspaceStore } from '$lib/stores'
|
||||
import FlowChatModelSettings from './FlowChatModelSettings.svelte'
|
||||
@@ -73,6 +74,21 @@
|
||||
// chat says what to go and do instead of offering controls that write nowhere.
|
||||
const modelGap = $derived(agentModelGap(modelWiring))
|
||||
|
||||
// Whether the reasoning registry has rules for the model in use. Only then does the
|
||||
// model button show a thinking control and own the effort input; on a provider it cannot
|
||||
// speak for — `customai` fronting any OpenAI-compatible endpoint, say — the field stays
|
||||
// in the Configure-inputs modal rather than being hidden behind a control never drawn.
|
||||
const effortKnown = $derived.by(() => {
|
||||
if (!modelWiring || modelWiring.whole) return false
|
||||
const pick = (field: 'model' | 'kind') => {
|
||||
const name = modelWiring.fields[field]
|
||||
return name ? effectiveInputs[name] : modelWiring.fixed[field]
|
||||
}
|
||||
const [kind, model] = [pick('kind'), pick('model')]
|
||||
if (typeof kind !== 'string' || !kind || typeof model !== 'string' || !model) return false
|
||||
return getReasoningCapability(kind as AIProvider, model).known
|
||||
})
|
||||
|
||||
// LocalStorage helpers
|
||||
const STORAGE_KEY_PREFIX = 'windmill_flow_chat_inputs_'
|
||||
|
||||
@@ -144,7 +160,7 @@
|
||||
attachmentsTarget: () => attachmentsTarget,
|
||||
workspace: () => chatWorkspace,
|
||||
canAttach: () => workspaceStorage.current,
|
||||
inputsShownInComposer: () => agentModelWiringInputs(modelWiring),
|
||||
inputsShownInComposer: () => agentModelWiringInputs(modelWiring, effortKnown),
|
||||
inputsSchema: () => additionalInputsSchema
|
||||
})
|
||||
setChatViewHost(chatHost)
|
||||
@@ -162,7 +178,7 @@
|
||||
// The paperclip's own condition, not half of it: an attachments input the composer
|
||||
// has no editor for — no object storage in the workspace, say — stays askable here.
|
||||
...(chatHost.supportsMessageAttachments && attachmentsTarget ? [attachmentsTarget.name] : []),
|
||||
...agentModelWiringInputs(modelWiring)
|
||||
...agentModelWiringInputs(modelWiring, effortKnown)
|
||||
])
|
||||
const properties = Object.fromEntries(
|
||||
Object.entries(additionalInputsSchema.properties ?? {}).filter(([key]) => !promoted.has(key))
|
||||
|
||||
@@ -148,21 +148,32 @@
|
||||
}
|
||||
})
|
||||
|
||||
/** Whether the registry can speak for this model at all. On a provider it has no rules
|
||||
* for the answer is "we do not know", and the composer neither shows a thinking control
|
||||
* nor touches the stored effort — the Configure-inputs modal keeps asking for it. */
|
||||
const effortKnown = $derived(
|
||||
!!provider &&
|
||||
typeof model === 'string' &&
|
||||
!!model &&
|
||||
getReasoningCapability(provider, model).known
|
||||
)
|
||||
|
||||
/**
|
||||
* The effort to write alongside a new model, which is `''` — the agent's "no effort" —
|
||||
* wherever that model has no such level. The composer owns this input, so it keeps it
|
||||
* runnable rather than leaving behind a level the provider would reject; the slider
|
||||
* shows the model cannot think, and the reader has nothing to clear by hand.
|
||||
* The effort to write alongside a new model: `''` — the agent's "no effort" — wherever
|
||||
* that model has no such level, so the composer never leaves behind a level the provider
|
||||
* would reject. Writes nothing where the registry cannot speak for the model, since
|
||||
* clearing a value on a guess would destroy the author's own default.
|
||||
*/
|
||||
function effortFor(nextModel: string | undefined): string {
|
||||
if (!provider || !nextModel) return ''
|
||||
return (
|
||||
carriedReasoning(
|
||||
typeof effort === 'string' ? effort : undefined,
|
||||
explicitOffToken(provider, nextModel) ?? '',
|
||||
getReasoningCapability(provider, nextModel)
|
||||
) ?? ''
|
||||
function effortPatch(nextModel: string | undefined): Partial<Record<ProviderField, any>> {
|
||||
if (!provider || !nextModel) return {}
|
||||
const capability = getReasoningCapability(provider, nextModel)
|
||||
if (!capability.known) return {}
|
||||
const carried = carriedReasoning(
|
||||
typeof effort === 'string' ? effort : undefined,
|
||||
explicitOffToken(provider, nextModel) ?? '',
|
||||
capability
|
||||
)
|
||||
return { reasoning_effort: carried ?? '' }
|
||||
}
|
||||
|
||||
function selectResource(path: string, picked: AIProvider) {
|
||||
@@ -171,9 +182,10 @@
|
||||
resource: `$res:${path}`,
|
||||
// The models of one provider mean nothing to another, and the new list only
|
||||
// arrives async, so there is nothing to carry the current one against — nor the
|
||||
// effort, which only means something against a model.
|
||||
// effort, which only means something against a model. Cleared only where the
|
||||
// registry can speak for the new provider, for the same reason as `effortPatch`.
|
||||
model: undefined,
|
||||
reasoning_effort: ''
|
||||
...(getReasoningCapability(picked, '').known ? { reasoning_effort: '' } : {})
|
||||
})
|
||||
}
|
||||
|
||||
@@ -222,7 +234,7 @@
|
||||
key: m,
|
||||
label: m,
|
||||
selected: m === model,
|
||||
onSelect: () => setFields({ model: m, reasoning_effort: effortFor(m) })
|
||||
onSelect: () => setFields({ model: m, ...effortPatch(m) })
|
||||
})),
|
||||
loading: models.loading,
|
||||
emptyMessage: provider ? 'No model available' : 'Pick a provider first'
|
||||
@@ -232,7 +244,7 @@
|
||||
// Offered as a slider only where the flow exposed it. When nothing is editable the
|
||||
// menu never opens, so passing it there only names the effort on the button.
|
||||
reasoning:
|
||||
(effortEditable || readOnly) && provider && typeof model === 'string' && model
|
||||
(effortEditable || readOnly) && effortKnown && provider && typeof model === 'string' && model
|
||||
? {
|
||||
provider,
|
||||
model,
|
||||
|
||||
@@ -142,11 +142,24 @@ describe('agentModelWiringInputs', () => {
|
||||
expect(agentModelWiringInputs(wiring)).toEqual(['m'])
|
||||
})
|
||||
|
||||
// The registry has rules per provider family and shrugs at the rest, so a `customai`
|
||||
// endpoint — which may well serve a thinking model — must keep its effort input askable
|
||||
// rather than hidden behind a slider the button never draws.
|
||||
it('keeps a reasoning_effort input where the registry cannot speak for the model', () => {
|
||||
const wiring = resolveAgentModelWiring([
|
||||
agent(
|
||||
`({ "kind": "customai", "resource": "$res:u/admin/custom", model: flow_input.m, reasoning_effort: flow_input.thinking })`
|
||||
)
|
||||
])
|
||||
expect(agentModelWiringInputs(wiring, false)).toEqual(['m'])
|
||||
expect(agentModelWiringInputs(wiring, true)?.sort()).toEqual(['m', 'thinking'])
|
||||
})
|
||||
|
||||
it('hides a kind input it writes with the resource', () => {
|
||||
const wiring = resolveAgentModelWiring([
|
||||
agent(`({ kind: flow_input.k, resource: flow_input.r, model: flow_input.m })`)
|
||||
])
|
||||
expect(agentModelWiringInputs(wiring)?.sort()).toEqual(['k', 'm', 'r'])
|
||||
expect(agentModelWiringInputs(wiring, true)?.sort()).toEqual(['k', 'm', 'r'])
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -294,18 +294,24 @@ export function agentModelGap(wiring: AgentModelWiring | undefined): string | un
|
||||
* resource leaves the button nothing to write it with, and hiding it would leave the run
|
||||
* without a provider kind and no way to supply one.
|
||||
*
|
||||
* `reasoning_effort` is unconditional by contrast. Once a model is chosen the button has a
|
||||
* control for it — the slider, or the row saying the model cannot think — and on a model
|
||||
* that cannot, the value is not a choice but a fact the button writes itself. Offering it in
|
||||
* the modal as well would invite setting a level the provider then rejects. Before a model
|
||||
* is chosen it is offered nowhere, which is the honest answer: an effort means nothing
|
||||
* until there is something to spend it on, and picking a model writes one.
|
||||
* `reasoning_effort` follows the same shape, keyed on whether the reasoning registry can
|
||||
* speak for the model in use. Where it can, the button owns the field — the slider, or the
|
||||
* row saying the model cannot think, on which the value is not a choice but a fact the
|
||||
* button writes itself — and offering it in the modal too would invite setting a level the
|
||||
* provider rejects. Where it cannot (a `customai` endpoint, say, which may well serve a
|
||||
* thinking model), the button shows no thinking control at all, so the modal has to keep
|
||||
* asking. Before a model is chosen nothing is known either, which is the same answer.
|
||||
*/
|
||||
export function agentModelWiringInputs(wiring: AgentModelWiring | undefined): string[] {
|
||||
export function agentModelWiringInputs(
|
||||
wiring: AgentModelWiring | undefined,
|
||||
/** Whether the registry can answer for the model in use. False when it cannot be told. */
|
||||
effortKnown: boolean = false
|
||||
): string[] {
|
||||
if (!wiring) return []
|
||||
if (wiring.whole) return [wiring.whole]
|
||||
const driven: ProviderField[] = ['resource', 'model', 'reasoning_effort']
|
||||
const driven: ProviderField[] = ['resource', 'model']
|
||||
if (wiring.fields.resource !== undefined) driven.push('kind')
|
||||
if (effortKnown) driven.push('reasoning_effort')
|
||||
return driven.map((field) => wiring.fields[field]).filter((name): name is string => !!name)
|
||||
}
|
||||
|
||||
|
||||
@@ -135,16 +135,24 @@ function toDisplayMessage(
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the turn a user message started came back unsuccessful. The answer is on
|
||||
* the messages that follow it, up to the next user message: an AI agent step or the
|
||||
* flow itself writes one with `success` false.
|
||||
* 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++) {
|
||||
if (messages[i].message_type === 'user') return false
|
||||
if (messages[i].success === false) return true
|
||||
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 false
|
||||
return last?.success === false
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -115,8 +115,9 @@ input, read by the agent.
|
||||
|
||||
**Wire the provider field by field, or the chat cannot change its model.** Each `provider` field fed
|
||||
by a flow input becomes a control in the composer — a provider picker, a model list, a thinking
|
||||
slider — while a field left static is fixed and shown read-only. `user_attachments` works the same
|
||||
way: point it at an s3-object input and the composer gets a paperclip.
|
||||
slider — while a field left static is fixed, and the composer draws no control for it.
|
||||
`user_attachments` works the same way: point it at an s3-object input and the composer gets a
|
||||
paperclip.
|
||||
|
||||
```json
|
||||
{
|
||||
|
||||
@@ -146,8 +146,9 @@ input, read by the agent.
|
||||
|
||||
**Wire the provider field by field, or the chat cannot change its model.** Each \`provider\` field fed
|
||||
by a flow input becomes a control in the composer — a provider picker, a model list, a thinking
|
||||
slider — while a field left static is fixed and shown read-only. \`user_attachments\` works the same
|
||||
way: point it at an s3-object input and the composer gets a paperclip.
|
||||
slider — while a field left static is fixed, and the composer draws no control for it.
|
||||
\`user_attachments\` works the same way: point it at an s3-object input and the composer gets a
|
||||
paperclip.
|
||||
|
||||
\`\`\`json
|
||||
{
|
||||
|
||||
@@ -203,8 +203,9 @@ input, read by the agent.
|
||||
|
||||
**Wire the provider field by field, or the chat cannot change its model.** Each `provider` field fed
|
||||
by a flow input becomes a control in the composer — a provider picker, a model list, a thinking
|
||||
slider — while a field left static is fixed and shown read-only. `user_attachments` works the same
|
||||
way: point it at an s3-object input and the composer gets a paperclip.
|
||||
slider — while a field left static is fixed, and the composer draws no control for it.
|
||||
`user_attachments` works the same way: point it at an s3-object input and the composer gets a
|
||||
paperclip.
|
||||
|
||||
```json
|
||||
{
|
||||
|
||||
@@ -115,8 +115,9 @@ input, read by the agent.
|
||||
|
||||
**Wire the provider field by field, or the chat cannot change its model.** Each `provider` field fed
|
||||
by a flow input becomes a control in the composer — a provider picker, a model list, a thinking
|
||||
slider — while a field left static is fixed and shown read-only. `user_attachments` works the same
|
||||
way: point it at an s3-object input and the composer gets a paperclip.
|
||||
slider — while a field left static is fixed, and the composer draws no control for it.
|
||||
`user_attachments` works the same way: point it at an s3-object input and the composer gets a
|
||||
paperclip.
|
||||
|
||||
```json
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user