mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-09 08:03:50 +00:00
perf(dynselect): only retrigger when helper args actually change (#9148)
* perf(dynselect): only retrigger when helper-script args actually change Parse the inline helper's signature with the existing WASM parser and restrict the form-arg diff to keys the helper actually consumes. Typing into unrelated fields no longer queues a dynselect job every second. Falls back to the previous full-args comparison when the helper is deployed or parsing fails. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(dynselect): avoid double helper-script fetch on mount usePromise defaults to loadInit=true, so refresh() ran before the JobLoader child was bound (firing a no-op pending promise) and the $effect then fired a second refresh once the bind:this resolved. Disable loadInit so the effect owns the single first call. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(dynselect): use parser directly instead of inferArgs inferArgs mutates a Schema object we never use and goes through a shared cache; when fed an empty schema for non-main entrypoints the caller cannot reliably read back the resulting properties. Add parseEntrypointArgs that just runs the parser and returns the parameter name Set (or undefined when unknown / unsupported / has rest args / function not found). DynamicInput uses that and keeps the previous params in flight while the next parse is computing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(dynselect): support deployed helpers in smart retrigger Add getHelperEntrypointArgs which dispatches on HelperScript.source: inline parses immediately; deployed fetches the script (or the flow's inline dyn-select code) once and caches per (workspace, kind, path, entrypoint). Without this the /scripts/get/* run view fell back to the full-args comparison and still retriggered on unrelated fields. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(dynselect): zero-arg helpers report empty deps, not unknown Codex review flagged that a valid zero-parameter entrypoint was being treated as "couldn't determine signature" and falling back to the full-args comparison. Distinguish "function found with no params" from "function not found" via the parser's auto_kind field — only the latter sets it, so empty args + auto_kind=null means a real zero-arg helper and we return an empty Set (no retrigger on unrelated fields). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
4d0f2c26a1
commit
dd19e52a84
@@ -25,6 +25,7 @@
|
||||
import { type DynamicInput } from '$lib/utils'
|
||||
import { deepEqual } from 'fast-equals'
|
||||
import { untrack } from 'svelte'
|
||||
import { getHelperEntrypointArgs } from '$lib/infer'
|
||||
|
||||
interface Props {
|
||||
value?: any
|
||||
@@ -48,7 +49,9 @@
|
||||
})
|
||||
|
||||
let resultJobLoader: JobLoader | undefined = $state()
|
||||
let _items = usePromise(getItemsFromOptions, { clearValueOnRefresh: false })
|
||||
// loadInit:false — the $effect below owns the first refresh once
|
||||
// resultJobLoader is bound; without this the promise is kicked off twice.
|
||||
let _items = usePromise(getItemsFromOptions, { clearValueOnRefresh: false, loadInit: false })
|
||||
let items = $derived(_items.value)
|
||||
|
||||
let filterText: string = $state('')
|
||||
@@ -125,9 +128,43 @@
|
||||
}, 1000)
|
||||
})
|
||||
|
||||
// Parameter names declared by the helper function. When known, we restrict
|
||||
// the change-detection to only those keys so typing in unrelated form fields
|
||||
// no longer retriggers the dynselect job. `undefined` means we couldn't
|
||||
// determine the signature → fall back to a full-args comparison.
|
||||
let helperParams = $state<Set<string> | undefined>(undefined)
|
||||
|
||||
$effect(() => {
|
||||
const script = helperScript
|
||||
const ep = entrypoint
|
||||
if (!script) {
|
||||
helperParams = undefined
|
||||
return
|
||||
}
|
||||
let cancelled = false
|
||||
void getHelperEntrypointArgs(script, ep || undefined).then((params) => {
|
||||
if (!cancelled) helperParams = params
|
||||
})
|
||||
return () => {
|
||||
cancelled = true
|
||||
}
|
||||
})
|
||||
|
||||
function filterArgs(args: Record<string, any> | undefined) {
|
||||
if (!args || !helperParams) return args
|
||||
const filtered: Record<string, any> = {}
|
||||
for (const k of helperParams) {
|
||||
if (k in args) filtered[k] = args[k]
|
||||
}
|
||||
return filtered
|
||||
}
|
||||
|
||||
$effect(() => {
|
||||
;[filterText, entrypoint, helperScript]
|
||||
if (resultJobLoader && (open || neverLoaded || !deepEqual(lastArgs, nargs))) {
|
||||
if (
|
||||
resultJobLoader &&
|
||||
(open || neverLoaded || !deepEqual(filterArgs(lastArgs), filterArgs(nargs)))
|
||||
) {
|
||||
neverLoaded = false
|
||||
lastArgs = $state.snapshot(otherArgs)
|
||||
_items.refresh()
|
||||
|
||||
@@ -7,7 +7,13 @@ import {
|
||||
} from '$lib/gen'
|
||||
import { get, writable } from 'svelte/store'
|
||||
import type { Schema, SupportedLanguage } from './common.js'
|
||||
import { emptySchema, getHubFlowIdFromPath, isHubFlowPath, sortObject } from './utils.js'
|
||||
import {
|
||||
type DynamicInput,
|
||||
emptySchema,
|
||||
getHubFlowIdFromPath,
|
||||
isHubFlowPath,
|
||||
sortObject
|
||||
} from './utils.js'
|
||||
import { tick } from 'svelte'
|
||||
|
||||
import initTsParser, { parse_deno, parse_outputs } from 'windmill-parser-wasm-ts'
|
||||
@@ -286,6 +292,88 @@ const SQL_LANGUAGES = [
|
||||
'duckdb'
|
||||
]
|
||||
|
||||
/**
|
||||
* Returns the parameter names of `entrypoint` in `code` (or `main` if not given),
|
||||
* or `undefined` if the function can't be found, the code can't be parsed, the
|
||||
* language isn't supported here, or the signature contains rest/keyword args
|
||||
* (in which case the callee should fall back to a conservative full comparison).
|
||||
*
|
||||
* Lighter than {@link inferArgs} — does not touch any schema.
|
||||
*/
|
||||
export async function parseEntrypointArgs(
|
||||
language: SupportedLanguage | 'bunnative' | undefined,
|
||||
code: string,
|
||||
entrypoint?: string
|
||||
): Promise<Set<string> | undefined> {
|
||||
if (!code) return undefined
|
||||
try {
|
||||
let sig: MainArgSignature
|
||||
if (language === 'python3') {
|
||||
await initWasmPython()
|
||||
sig = JSON.parse(parse_python(code, entrypoint))
|
||||
} else if (
|
||||
language === 'deno' ||
|
||||
language === 'nativets' ||
|
||||
language === 'bun' ||
|
||||
language === 'bunnative'
|
||||
) {
|
||||
await initWasmTs()
|
||||
sig = JSON.parse(parse_deno(code, entrypoint))
|
||||
} else {
|
||||
return undefined
|
||||
}
|
||||
if (sig.type === 'Invalid') return undefined
|
||||
if (sig.star_args || sig.star_kwargs) return undefined
|
||||
if (!Array.isArray(sig.args)) return undefined
|
||||
// The parser sets auto_kind when no matching entrypoint function was
|
||||
// found — empty args in that case means "unknown signature", not
|
||||
// "function takes no params", so we fall back to a full comparison.
|
||||
if (sig.args.length === 0 && sig.auto_kind != null) return undefined
|
||||
return new Set(sig.args.map((a) => a.name))
|
||||
} catch {
|
||||
return undefined
|
||||
}
|
||||
}
|
||||
|
||||
const helperEntrypointCache = new Map<string, Set<string> | undefined>()
|
||||
|
||||
/**
|
||||
* Resolves a {@link DynamicInput.HelperScript} to its entrypoint parameter
|
||||
* names. For deployed helpers it fetches the script (or the flow's inline
|
||||
* dyn-select code) once and caches the result per workspace+path+entrypoint.
|
||||
*/
|
||||
export async function getHelperEntrypointArgs(
|
||||
helper: DynamicInput.HelperScript,
|
||||
entrypoint?: string
|
||||
): Promise<Set<string> | undefined> {
|
||||
if (helper.source === 'inline') {
|
||||
return parseEntrypointArgs(helper.lang, helper.code, entrypoint)
|
||||
}
|
||||
const workspace = get(workspaceStore)
|
||||
if (!workspace) return undefined
|
||||
const cacheKey = `${workspace}::${helper.runnable_kind}::${helper.path}::${entrypoint ?? ''}`
|
||||
if (helperEntrypointCache.has(cacheKey)) return helperEntrypointCache.get(cacheKey)
|
||||
let result: Set<string> | undefined
|
||||
try {
|
||||
if (helper.runnable_kind === 'script') {
|
||||
const script = await ScriptService.getScriptByPath({ workspace, path: helper.path })
|
||||
result = await parseEntrypointArgs(script.language, script.content ?? '', entrypoint)
|
||||
} else {
|
||||
const flow = await FlowService.getFlowByPath({ workspace, path: helper.path })
|
||||
const schema = flow.schema as Record<string, unknown> | undefined
|
||||
const code = schema?.['x-windmill-dyn-select-code']
|
||||
const lang = schema?.['x-windmill-dyn-select-lang']
|
||||
if (typeof code === 'string' && typeof lang === 'string') {
|
||||
result = await parseEntrypointArgs(lang as SupportedLanguage, code, entrypoint)
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
result = undefined
|
||||
}
|
||||
helperEntrypointCache.set(cacheKey, result)
|
||||
return result
|
||||
}
|
||||
|
||||
export async function inferArgs(
|
||||
language: SupportedLanguage | 'bunnative' | undefined,
|
||||
code: string,
|
||||
|
||||
Reference in New Issue
Block a user