mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-12 00:06:14 +00:00
fix: carry loop web search availability into every prompt rebuild
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
eea13810e4
commit
a3872d3cf3
@@ -180,11 +180,9 @@ export async function runGlobalEval(
|
||||
systemMessage: prepareGlobalSystemMessage(undefined, {
|
||||
user: options.user,
|
||||
previewTools: options.sessionChat ?? false,
|
||||
// Derived from the model under test rather than left to the production
|
||||
// default, which reads the copilot model store the harness deliberately
|
||||
// leaves empty — that would resolve false and hide guidance the benchmarked
|
||||
// provider does serve. Mirrors runChatLoop's gate so a Gemini or DeepSeek
|
||||
// run is never told to use a tool the loop will not hand it.
|
||||
// Mirrors runChatLoop's gate. The production default reads the copilot model
|
||||
// store, which the harness leaves empty, so it would hide guidance every
|
||||
// benchmarked provider actually serves.
|
||||
webSearch: providerSupportsWebSearch(options.provider),
|
||||
}),
|
||||
userMessage: prepareGlobalUserMessage(
|
||||
|
||||
@@ -1846,7 +1846,8 @@ export class AIChatManager {
|
||||
private configureGlobalMode = () => {
|
||||
const systemMessage = prepareGlobalSystemMessage(getCustomPromptParts(AIMode.GLOBAL), {
|
||||
previewTools: this.isSessionChat,
|
||||
skills: this.globalSkills
|
||||
skills: this.globalSkills,
|
||||
webSearch: this.globalWebSearchAdvertised
|
||||
})
|
||||
const sessionCtx = this.sessionContextResolver?.()
|
||||
if (sessionCtx) {
|
||||
@@ -1929,7 +1930,10 @@ export class AIChatManager {
|
||||
}
|
||||
const systemMessage = prepareGlobalSystemMessage(getCustomPromptParts(AIMode.GLOBAL), {
|
||||
previewTools: this.isSessionChat,
|
||||
skills: this.globalSkills
|
||||
skills: this.globalSkills,
|
||||
// Carry the loop's observed availability: re-deriving would lose a runtime
|
||||
// rejection, which the static provider/settings gates cannot see.
|
||||
webSearch: this.globalWebSearchAdvertised
|
||||
})
|
||||
// Preserve the session-state and active pipeline-editor augmentations that
|
||||
// configureGlobalMode adds — otherwise update_user_instructions (which calls
|
||||
@@ -3151,6 +3155,9 @@ export class AIChatManager {
|
||||
addedMessages: collectedMessages,
|
||||
onWebSearchUnavailable: () => {
|
||||
webSearchUnavailable = true
|
||||
// The loop drops the tool for the rest of this workspace+model; drop the
|
||||
// guidance with it so the retry and later turns stop advertising it.
|
||||
this.syncGlobalWebSearchGuidance(false)
|
||||
}
|
||||
})
|
||||
const wasAborted = this.abortController?.signal.aborted ?? false
|
||||
|
||||
@@ -276,6 +276,41 @@ describe('AIChatManager global skills', () => {
|
||||
expect(manager.systemMessage.content).not.toContain('parent-skill')
|
||||
})
|
||||
|
||||
// The loop drops the web-search tool on a provider switch or a runtime rejection.
|
||||
// Rebuilding the prompt from the static gates alone would re-advertise it, and the
|
||||
// recorded flag then suppresses any further rebuild — so it would never come back.
|
||||
it('tracks the loop web search availability, and keeps a rebuild from resurrecting it', async () => {
|
||||
const sendWith = async (manager: AIChatManager, webSearch: boolean) => {
|
||||
let sent = ''
|
||||
mocks.runChatLoop.mockImplementation(async (config: any) => {
|
||||
await config.onBeforeIteration?.(
|
||||
[],
|
||||
undefined,
|
||||
{ provider: 'openai', model: 'gpt-5' },
|
||||
webSearch
|
||||
)
|
||||
sent = config.systemMessage.content
|
||||
const message = { role: 'assistant' as const, content: 'done' }
|
||||
config.addedMessages?.push(message)
|
||||
return {
|
||||
addedMessages: [message],
|
||||
tokenUsage: { prompt: 0, completion: 0, total: 0 },
|
||||
hitMaxIterations: false
|
||||
}
|
||||
})
|
||||
await manager.sendRequest({ instructions: 'go', mode: AIMode.GLOBAL })
|
||||
return sent
|
||||
}
|
||||
|
||||
const manager = new AIChatManager()
|
||||
expect(await sendWith(manager, true)).toContain('search the web')
|
||||
expect(await sendWith(manager, false)).not.toContain('search the web')
|
||||
|
||||
// A later rebuild for an unrelated reason must not resurrect the guidance.
|
||||
manager.rebuildGlobalSystemMessage()
|
||||
expect(manager.systemMessage.content).not.toContain('search the web')
|
||||
})
|
||||
|
||||
it('expands a leading slash skill command for the model while preserving the displayed text', async () => {
|
||||
mocks.listAiSkills.mockResolvedValue([
|
||||
{ name: 'review-code', description: 'review code for bugs' }
|
||||
|
||||
@@ -339,6 +339,43 @@ describe('runChatLoop onBeforeIteration web search sync', () => {
|
||||
expect(seen).toEqual([true, false])
|
||||
})
|
||||
|
||||
it('sends the corrected system message on the fallback retry, not the rejected one', async () => {
|
||||
let systemMessage: ChatLoopConfig['systemMessage'] = {
|
||||
role: 'system',
|
||||
content: 'search the web'
|
||||
}
|
||||
|
||||
mocks.getOpenAIResponsesCompletion
|
||||
.mockRejectedValueOnce(
|
||||
Object.assign(new Error("Hosted tool 'web_search' is not supported with this model"), {
|
||||
status: 400,
|
||||
error: { type: 'invalid_request_error' }
|
||||
})
|
||||
)
|
||||
.mockResolvedValue({})
|
||||
|
||||
await runChatLoop(
|
||||
createConfig({
|
||||
workspace: `workspace-${randomUUID()}`,
|
||||
getSystemMessage: () => systemMessage,
|
||||
// Production drops the guidance here; the retry must pick that up.
|
||||
onWebSearchUnavailable: () => {
|
||||
systemMessage = { role: 'system', content: 'no web search' }
|
||||
}
|
||||
})
|
||||
)
|
||||
|
||||
expect(mocks.getOpenAIResponsesCompletion).toHaveBeenCalledTimes(2)
|
||||
expect(mocks.getOpenAIResponsesCompletion.mock.calls[0][0][0]).toEqual({
|
||||
role: 'system',
|
||||
content: 'search the web'
|
||||
})
|
||||
expect(mocks.getOpenAIResponsesCompletion.mock.calls[1][0][0]).toEqual({
|
||||
role: 'system',
|
||||
content: 'no web search'
|
||||
})
|
||||
})
|
||||
|
||||
it('sends the system message the callback rewrote on this iteration, not the next', async () => {
|
||||
let systemMessage: ChatLoopConfig['systemMessage'] = { role: 'system', content: 'stale' }
|
||||
const config = createConfig({
|
||||
|
||||
@@ -71,11 +71,9 @@ export interface ChatLoopConfig {
|
||||
*/
|
||||
addedMessages?: ChatCompletionMessageParam[]
|
||||
/** Called before each iteration (e.g. to refresh tool schemas, or to record
|
||||
* which model the iteration is about to use). `webSearch` is the effective
|
||||
* value for this iteration — static gates and the runtime probe both applied —
|
||||
* so a caller whose system prompt advertises web search can resync here. The
|
||||
* system message is read after this returns, so such a resync lands on this
|
||||
* iteration rather than the next. */
|
||||
* which model it is about to use). `webSearch` is this iteration's effective
|
||||
* value, and the system message is read after this returns, so a caller whose
|
||||
* prompt advertises web search can resync it in time for this request. */
|
||||
onBeforeIteration?: (
|
||||
tools: Tool<any>[],
|
||||
helpers: any,
|
||||
@@ -388,11 +386,18 @@ export async function runChatLoop(config: ChatLoopConfig): Promise<ChatLoopResul
|
||||
const visibleMessages = modelSupportsVision(modelProvider.provider, modelProvider.model)
|
||||
? boundImagePartBytes(messages)
|
||||
: stripImagePartsFromMessages(messages)
|
||||
const messageParams = [
|
||||
systemMessage,
|
||||
const sanitizedMessages = [
|
||||
...sanitizeToolCallArguments(visibleMessages),
|
||||
...(pendingUserMessage ? [pendingUserMessage] : [])
|
||||
]
|
||||
// Rebuilt per attempt so a web-search fallback retry picks up a system message
|
||||
// the rejection just corrected; reusing the first attempt's array would resend
|
||||
// guidance for a tool the retry deliberately drops.
|
||||
const messageParamsFor = (currentSystemMessage: ChatCompletionSystemMessageParam) => [
|
||||
currentSystemMessage,
|
||||
...sanitizedMessages
|
||||
]
|
||||
const messageParams = messageParamsFor(systemMessage)
|
||||
const toolDefs = tools.map((t) => t.def)
|
||||
const parseOptions = { workspace, provider: modelProvider.provider }
|
||||
|
||||
@@ -414,7 +419,7 @@ export async function runChatLoop(config: ChatLoopConfig): Promise<ChatLoopResul
|
||||
|
||||
const runOpenAIResponses = async (useWebSearch: boolean): Promise<boolean> => {
|
||||
const completion = await getOpenAIResponsesCompletion(
|
||||
messageParams,
|
||||
messageParamsFor(useWebSearch === webSearch ? systemMessage : config.systemMessage),
|
||||
abortController,
|
||||
toolDefs,
|
||||
{
|
||||
@@ -517,7 +522,10 @@ export async function runChatLoop(config: ChatLoopConfig): Promise<ChatLoopResul
|
||||
}
|
||||
} else if (isAnthropic) {
|
||||
const runAnthropic = async (useWebSearch: boolean): Promise<boolean> => {
|
||||
const completion = await getAnthropicCompletion(messageParams, abortController, toolDefs, {
|
||||
const attemptParams = messageParamsFor(
|
||||
useWebSearch === webSearch ? systemMessage : config.systemMessage
|
||||
)
|
||||
const completion = await getAnthropicCompletion(attemptParams, abortController, toolDefs, {
|
||||
forceModelProvider: modelProvider,
|
||||
anthropicClient: clients.anthropic,
|
||||
webSearch: useWebSearch,
|
||||
|
||||
Reference in New Issue
Block a user