mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 08:02:38 +00:00
fix: let the flow chat take a typed model id and keep a shared thinking input editable
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
b076f0ddbe
commit
0de006f47d
@@ -48,8 +48,8 @@
|
||||
})
|
||||
: undefined
|
||||
)
|
||||
// The stops, the one in use and the trigger's suffix are decided together, in one
|
||||
// tested place: they have to agree, and three rounds of review found them disagreeing.
|
||||
// The stops, the one in use and the trigger's suffix are decided together, in one tested
|
||||
// place: a stop the slider shows as `off` must not read as the provider's `none` on the button.
|
||||
const display = $derived(reasoningDisplay(reasoning, capability, effective))
|
||||
const stops = $derived(display.stops)
|
||||
const currentStop = $derived(display.currentStop)
|
||||
@@ -117,7 +117,49 @@
|
||||
</div>
|
||||
{/snippet}
|
||||
|
||||
{#snippet section(sec: ChoiceSection, item: MeltItem)}
|
||||
{#snippet typedField(
|
||||
value: string,
|
||||
placeholder: string,
|
||||
onCommit: (value: string) => void,
|
||||
close: () => void
|
||||
)}
|
||||
{#key value}
|
||||
<TextInput
|
||||
size="sm"
|
||||
{value}
|
||||
inputProps={{
|
||||
placeholder,
|
||||
onchange: (e) => onCommit(e.currentTarget.value.trim()),
|
||||
// Capture, not bubble: Svelte delegates `keydown` to the root, which sits above the
|
||||
// menu — so a bubble handler here would run only after melt's own listener had read
|
||||
// the key as typeahead and moved focus. A capture key is not delegatable, so this
|
||||
// becomes a real listener on the input and sees the event first.
|
||||
onkeydowncapture: (e) => {
|
||||
// Escape cancels: let it reach the menu with the value untouched.
|
||||
if (e.key === 'Escape') return
|
||||
// Tab closes the menu, unmounting this field before focus moves, so no change
|
||||
// event would ever fire. Commit on the way past.
|
||||
if (e.key === 'Tab') {
|
||||
onCommit(e.currentTarget.value.trim())
|
||||
return
|
||||
}
|
||||
// Enter means done: commit and close, rather than leaving the menu open around a
|
||||
// field the commit is about to rebuild.
|
||||
if (e.key === 'Enter') {
|
||||
e.preventDefault()
|
||||
onCommit(e.currentTarget.value.trim())
|
||||
close()
|
||||
return
|
||||
}
|
||||
// Everything else is typing; the menu reads loose keys as typeahead.
|
||||
e.stopPropagation()
|
||||
}
|
||||
}}
|
||||
/>
|
||||
{/key}
|
||||
{/snippet}
|
||||
|
||||
{#snippet section(sec: ChoiceSection, item: MeltItem, close: () => void)}
|
||||
<div class="px-3 pt-1.5 pb-1 text-2xs uppercase tracking-wide text-secondary">{sec.label}</div>
|
||||
{#if sec.loading}
|
||||
<div class="flex items-center gap-2 px-3 py-1.5 text-tertiary">
|
||||
@@ -140,6 +182,19 @@
|
||||
{/each}
|
||||
</div>
|
||||
{/if}
|
||||
{#if sec.custom && !sec.loading}
|
||||
{@const custom = sec.custom}
|
||||
<div class="px-3 pt-1 pb-1.5">
|
||||
{@render typedField(
|
||||
'',
|
||||
custom.placeholder,
|
||||
(value) => {
|
||||
if (value) custom.onCommit(value)
|
||||
},
|
||||
close
|
||||
)}
|
||||
</div>
|
||||
{/if}
|
||||
{/snippet}
|
||||
|
||||
{#snippet rows(items: Item[], item: MeltItem, builders: MeltBuilders)}
|
||||
@@ -188,7 +243,7 @@
|
||||
{/if}
|
||||
{#each config.sections ?? [] as sec (sec.label)}
|
||||
<div class={BLOCK_CLASS}>
|
||||
{@render section(sec, item)}
|
||||
{@render section(sec, item, close)}
|
||||
</div>
|
||||
{/each}
|
||||
{#if reasoning}
|
||||
@@ -213,41 +268,7 @@
|
||||
would be a guess, and offering nothing would leave it settable nowhere. -->
|
||||
<div class="px-3 pt-1 pb-1.5">
|
||||
<div class="text-2xs uppercase tracking-wide text-secondary mb-1">Thinking</div>
|
||||
{#key reasoning.value}
|
||||
<TextInput
|
||||
size="sm"
|
||||
value={reasoning.value ?? ''}
|
||||
inputProps={{
|
||||
placeholder: 'none',
|
||||
onchange: (e) => reasoning?.onSelect(e.currentTarget.value.trim()),
|
||||
// Capture, not bubble: Svelte delegates `keydown` to the root, which sits
|
||||
// above the menu — so a bubble handler here would run only after melt's own
|
||||
// listener had read the key as typeahead and moved focus. A capture key is
|
||||
// not delegatable, so this becomes a real listener on the input and sees the
|
||||
// event first.
|
||||
onkeydowncapture: (e) => {
|
||||
// Escape cancels: let it reach the menu with the value untouched.
|
||||
if (e.key === 'Escape') return
|
||||
// Tab closes the menu, unmounting this field before focus moves, so no
|
||||
// change event would ever fire. Commit on the way past.
|
||||
if (e.key === 'Tab') {
|
||||
reasoning?.onSelect(e.currentTarget.value.trim())
|
||||
return
|
||||
}
|
||||
// Enter means done: commit and close, rather than leaving the menu open
|
||||
// around a field the commit is about to rebuild.
|
||||
if (e.key === 'Enter') {
|
||||
e.preventDefault()
|
||||
reasoning?.onSelect(e.currentTarget.value.trim())
|
||||
close()
|
||||
return
|
||||
}
|
||||
// Everything else is typing; the menu reads loose keys as typeahead.
|
||||
e.stopPropagation()
|
||||
}
|
||||
}}
|
||||
/>
|
||||
{/key}
|
||||
{@render typedField(reasoning.value ?? '', 'none', reasoning.onSelect, close)}
|
||||
<div class="text-2xs text-tertiary mt-1">
|
||||
Windmill has no thinking levels for this provider — type what it accepts.
|
||||
</div>
|
||||
|
||||
@@ -31,8 +31,8 @@
|
||||
|
||||
/**
|
||||
* A `current` naming no stop is a real state, not a missing one: an agent that leaves the
|
||||
* effort unset sends nothing and the provider decides. Three things follow, and each has
|
||||
* been got wrong on its own — keep them together.
|
||||
* effort unset sends nothing and the provider decides. Three things follow, and they only
|
||||
* hold together.
|
||||
*
|
||||
* The thumb rests at the start, because a range input always has one somewhere, and
|
||||
* `overrideLabel` is what tells the reader this is not the lowest stop. The track is
|
||||
|
||||
@@ -129,8 +129,7 @@ describe('reasoningDisplay', () => {
|
||||
describe('carriedReasoning', () => {
|
||||
const cap = (model: string) => getReasoningCapability('openai', model)
|
||||
|
||||
// The bug this exists for: picking a model that cannot think left the old level in the
|
||||
// flow input, and the run sent it anyway.
|
||||
// A level carried onto a model that cannot think stays in the flow input, and the run sends it.
|
||||
it('drops a level the new model does not have', () => {
|
||||
expect(carriedReasoning('high', '', cap('gpt-4o'))).toBeUndefined()
|
||||
expect(carriedReasoning('xhigh', '', cap('gpt-5.1'))).toBeUndefined()
|
||||
|
||||
@@ -31,6 +31,11 @@ export type ChoiceSection = {
|
||||
/** Shown when the list is empty and settled. */
|
||||
emptyMessage?: string
|
||||
maxHeight?: string
|
||||
/**
|
||||
* A typed entry under the options, for a value the list does not hold: an endpoint with no
|
||||
* listing, or a model newer than the catalogue. An empty entry commits nothing.
|
||||
*/
|
||||
custom?: { placeholder: string; onCommit: (value: string) => void }
|
||||
}
|
||||
|
||||
export type ChatModelSettingsConfig = {
|
||||
|
||||
@@ -17,6 +17,7 @@
|
||||
agentModelGap,
|
||||
composerOwnedInputs,
|
||||
resolveAgentModelWiring,
|
||||
showsModelButton,
|
||||
withoutRejectedEffort
|
||||
} from './agentChatInputs'
|
||||
|
||||
@@ -61,6 +62,7 @@
|
||||
// An agent with nothing to call cannot answer, and the composer cannot fix it, so the
|
||||
// chat says what to go and do instead of offering controls that write nowhere.
|
||||
const modelGap = $derived(agentModelGap(modelWiring))
|
||||
const showModelButton = $derived(showsModelButton(modelWiring))
|
||||
|
||||
// LocalStorage helpers
|
||||
const STORAGE_KEY_PREFIX = 'windmill_flow_chat_inputs_'
|
||||
@@ -249,7 +251,7 @@
|
||||
{/if}
|
||||
</div>
|
||||
{/if}
|
||||
{#if modelWiring}
|
||||
{#if modelWiring && showModelButton}
|
||||
<!-- `runInputs`, not `effectiveInputs`: a stored effort the model rejects is dropped
|
||||
before the run, and the button must not name one the run will not send. -->
|
||||
<FlowChatModelSettings
|
||||
@@ -283,7 +285,7 @@
|
||||
hideModeSelector
|
||||
{wideLayout}
|
||||
{emptyHint}
|
||||
footerSettings={modalSchema || modelWiring ? footerSettings : undefined}
|
||||
footerSettings={modalSchema || showModelButton ? footerSettings : undefined}
|
||||
placeholder="Send a message to run the flow"
|
||||
disabled={deploymentInProgress || !!modelGap}
|
||||
disabledMessage={deploymentInProgress ? 'Deployment in progress' : (modelGap ?? '')}
|
||||
|
||||
@@ -23,7 +23,11 @@
|
||||
import type { Item } from '$lib/utils'
|
||||
import { Plug, Plus } from 'lucide-svelte'
|
||||
import { resource } from 'runed'
|
||||
import type { AgentModelWiring, ProviderField } from './agentChatInputs'
|
||||
import {
|
||||
composerDrivesEffort,
|
||||
type AgentModelWiring,
|
||||
type ProviderField
|
||||
} from './agentChatInputs'
|
||||
|
||||
interface Props {
|
||||
wiring: AgentModelWiring
|
||||
@@ -59,7 +63,9 @@
|
||||
|
||||
const resourceEditable = $derived(editable('resource'))
|
||||
const modelEditable = $derived(editable('model'))
|
||||
const effortEditable = $derived(editable('reasoning_effort'))
|
||||
const effortEditable = $derived(composerDrivesEffort(wiring))
|
||||
// Wired, but left to the Configure-inputs modal: the button has no model to place it on.
|
||||
const effortInModal = $derived(wiring.fields.reasoning_effort !== undefined && !effortEditable)
|
||||
// Nothing to write: the flow fixes the lot, so the button names it and opens nothing.
|
||||
const readOnly = $derived(!resourceEditable && !modelEditable && !effortEditable)
|
||||
|
||||
@@ -227,32 +233,43 @@
|
||||
onSelect: () => setFields({ model: m, ...effortPatch(m) })
|
||||
})),
|
||||
loading: models.loading,
|
||||
emptyMessage: provider ? 'No model available' : 'Pick a provider first'
|
||||
emptyMessage: provider ? 'No model listed' : 'Pick a provider first',
|
||||
// The step's own provider picker takes any model id, and the modal does not
|
||||
// ask for this input: without a typed entry, an endpoint that lists nothing
|
||||
// leaves the run with no model.
|
||||
custom: provider
|
||||
? {
|
||||
placeholder: 'Custom model id',
|
||||
onCommit: (m) => setFields({ model: m, ...effortPatch(m) })
|
||||
}
|
||||
: undefined
|
||||
}
|
||||
]
|
||||
: undefined,
|
||||
// Always present, whatever we can say about it: the run uses an effort either way, and
|
||||
// the control is the only place it can be read or set. What varies is the state it
|
||||
// renders — a ladder, a typed token, why there is none, or what the flow fixed.
|
||||
reasoning: {
|
||||
provider,
|
||||
model: typeof model === 'string' && model ? model : undefined,
|
||||
value: typeof effort === 'string' ? effort : undefined,
|
||||
// An agent writes the provider-native token straight into its step, so there is no
|
||||
// sentinel to translate later. Where a model disables by omission instead, the empty
|
||||
// string is that off: the run reads an empty `reasoning_effort` as absent
|
||||
// (types.rs `get_reasoning_effort`).
|
||||
offToken:
|
||||
provider && typeof model === 'string' && model
|
||||
? (explicitOffToken(provider, model) ?? '')
|
||||
: '',
|
||||
// An agent step omits `reasoning_effort` when it is unset, so the provider picks —
|
||||
// naming a level would claim something the run does not do.
|
||||
sendsDefaultWhenUnset: false,
|
||||
writable: effortEditable,
|
||||
typedWhenUnknown: true,
|
||||
onSelect: (token) => setFields({ reasoning_effort: token })
|
||||
}
|
||||
// Present whatever we can say about it, since the run uses an effort either way: a ladder,
|
||||
// a typed token, why there is none, or what the flow fixed. Absent only when the modal
|
||||
// is the effort's editor.
|
||||
reasoning: effortInModal
|
||||
? undefined
|
||||
: {
|
||||
provider,
|
||||
model: typeof model === 'string' && model ? model : undefined,
|
||||
value: typeof effort === 'string' ? effort : undefined,
|
||||
// An agent writes the provider-native token straight into its step, so there is no
|
||||
// sentinel to translate later. Where a model disables by omission instead, the empty
|
||||
// string is that off: the run reads an empty `reasoning_effort` as absent
|
||||
// (types.rs `get_reasoning_effort`).
|
||||
offToken:
|
||||
provider && typeof model === 'string' && model
|
||||
? (explicitOffToken(provider, model) ?? '')
|
||||
: '',
|
||||
// An agent step omits `reasoning_effort` when it is unset, so the provider picks —
|
||||
// naming a level would claim something the run does not do.
|
||||
sendsDefaultWhenUnset: false,
|
||||
writable: effortEditable,
|
||||
typedWhenUnknown: true,
|
||||
onSelect: (token) => setFields({ reasoning_effort: token })
|
||||
}
|
||||
})
|
||||
</script>
|
||||
|
||||
|
||||
@@ -5,6 +5,7 @@ import {
|
||||
composerOwnedInputs,
|
||||
parseProviderTransform,
|
||||
resolveAgentModelWiring,
|
||||
showsModelButton,
|
||||
withoutRejectedEffort
|
||||
} from './agentChatInputs'
|
||||
import type { FlowModule } from '$lib/gen'
|
||||
@@ -113,9 +114,9 @@ describe('agentModelWiringInputs', () => {
|
||||
expect(agentModelWiringInputs(wiring)?.sort()).toEqual(['k', 'm', 'r'])
|
||||
})
|
||||
|
||||
// The button always draws a thinking control — a ladder, a typed token, or why there is
|
||||
// neither — so a wired effort is the button's whatever the registry knows about the model.
|
||||
it('always claims a wired reasoning_effort, whatever the provider', () => {
|
||||
// A thinking control is usable whatever the registry knows about the model — a ladder, a
|
||||
// typed token, or why there is neither — so a wired effort is the button's there.
|
||||
it('claims a wired reasoning_effort whatever the provider', () => {
|
||||
const custom = resolveAgentModelWiring([
|
||||
agent(
|
||||
`({ "kind": "customai", "resource": "$res:u/admin/custom", model: flow_input.m, reasoning_effort: flow_input.thinking })`
|
||||
@@ -130,10 +131,22 @@ describe('agentModelWiringInputs', () => {
|
||||
])
|
||||
expect(agentModelWiringInputs(known)).toEqual(['thinking'])
|
||||
})
|
||||
|
||||
// Agents on different fixed models leave the button no model to place the effort on, so
|
||||
// it would say "Pick a model first" with nothing to pick. The modal keeps the input.
|
||||
it('leaves a shared effort to the modal when the agents fix different models', () => {
|
||||
const resource = `"kind": "anthropic", "resource": "$res:u/admin/claude"`
|
||||
const wiring = resolveAgentModelWiring([
|
||||
agent(`({ ${resource}, "model": "claude-sonnet-5", reasoning_effort: flow_input.thinking })`),
|
||||
agent(`({ ${resource}, "model": "claude-opus-5", reasoning_effort: flow_input.thinking })`)
|
||||
])
|
||||
expect(wiring?.fields.reasoning_effort).toBe('thinking')
|
||||
expect(agentModelWiringInputs(wiring)).toEqual([])
|
||||
expect(showsModelButton(wiring)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
// The modal is whatever this does not return, so the two can no longer disagree about an
|
||||
// input — and what a control can do *right now* is deliberately not part of the answer.
|
||||
// The modal is whatever this does not return, so the two cannot disagree about an input.
|
||||
describe('composerOwnedInputs', () => {
|
||||
const wiring = () =>
|
||||
resolveAgentModelWiring([
|
||||
|
||||
@@ -322,19 +322,41 @@ 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` needs no such condition: the button always draws a thinking control,
|
||||
* whatever it can say about the model — a ladder, a typed token, or why there is neither — so
|
||||
* a wired effort is always the button's. Asking for it in the modal as well would be a second
|
||||
* editor for a field that already has one.
|
||||
* `reasoning_effort` is the button's only where it has a model to place the effort against
|
||||
* (see `composerDrivesEffort`).
|
||||
*/
|
||||
export function agentModelWiringInputs(wiring: AgentModelWiring | undefined): 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 (composerDrivesEffort(wiring)) driven.push('reasoning_effort')
|
||||
return driven.map((field) => wiring.fields[field]).filter((name): name is string => !!name)
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the model button draws the thinking control for a wired effort.
|
||||
*
|
||||
* Every thinking state but one is usable: a ladder, a typed token, or why the model has
|
||||
* neither. The exception is a model the button cannot name, as when agents share an effort
|
||||
* input but fix different models. It would say "Pick a model first" with no model to pick,
|
||||
* so the effort stays with the modal.
|
||||
*/
|
||||
export function composerDrivesEffort(wiring: AgentModelWiring): boolean {
|
||||
if (wiring.whole) return true
|
||||
if (wiring.fields.reasoning_effort === undefined) return false
|
||||
return wiring.fields.model !== undefined || !agentFieldEmpty(wiring, 'model')
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the composer draws a model button at all: something to write, or a fixed model to
|
||||
* name. Agents that fix different models leave neither.
|
||||
*/
|
||||
export function showsModelButton(wiring: AgentModelWiring | undefined): boolean {
|
||||
if (!wiring) return false
|
||||
return agentModelWiringInputs(wiring).length > 0 || !agentFieldEmpty(wiring, 'model')
|
||||
}
|
||||
|
||||
/**
|
||||
* The flow inputs the composer edits, and therefore the ones the Configure-inputs modal must
|
||||
* not ask for. The modal is whatever is left, so this is the single answer to "who edits
|
||||
|
||||
Reference in New Issue
Block a user