mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-20 08:03:28 +00:00
fix(pipelines): dedupe inferred-lineage overlay against accumulated edges; first draft teardown still captures reads
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
dc7bdd9fac
commit
237aaf9120
@@ -54,6 +54,10 @@
|
||||
// `selection` when present. Used by the pipeline + menu so a new
|
||||
// pipeline script opens inline instead of navigating to /scripts/add.
|
||||
draftScript?: Script | undefined
|
||||
// Whether the draft entry behind `draftScript` has captured its read
|
||||
// lineage (`inputAssets != undefined`). Gates the teardown skip below: a
|
||||
// just-seeded draft must still emit once even with an untouched buffer.
|
||||
draftInputsCaptured?: boolean
|
||||
// Local-dev preview (`/pipeline_dev`): resolve a selected node to its
|
||||
// working-tree content instead of fetching the deployed script (there is
|
||||
// none — the pipeline is local-only). Returns a read-only `Script` so the
|
||||
@@ -261,6 +265,7 @@
|
||||
let {
|
||||
selection,
|
||||
draftScript,
|
||||
draftInputsCaptured = false,
|
||||
resolveLocalScript,
|
||||
localScriptsVersion,
|
||||
workspace,
|
||||
@@ -525,19 +530,27 @@
|
||||
const origAtRegister = isDraftRun ? undefined : scriptRes.current
|
||||
if (!isDraftRun && !origAtRegister) return
|
||||
const captured = script
|
||||
// Untracked — reading `.content` in the effect body would re-run (and
|
||||
// emit) on every keystroke, the exact per-key loop the header comment
|
||||
// forbids.
|
||||
// Untracked — reading `.content` (or the draft entry's fields) in the
|
||||
// effect body would re-run (and emit) on every keystroke, the exact
|
||||
// per-key loop the header comment forbids.
|
||||
const contentAtRegister = untrack(() => captured.content ?? '')
|
||||
// Whether the entry this registration rendered had already captured its
|
||||
// read lineage. A just-seeded draft hasn't (inputAssets undefined): its
|
||||
// first teardown must emit even with an untouched buffer, or the
|
||||
// now-inactive draft has no reads to render.
|
||||
const capturedHadReads = untrack(() => draftInputsCaptured)
|
||||
return () => {
|
||||
// Draft runs: only emit when the user actually typed into THIS clone.
|
||||
// When the drafts entry is rewritten externally while the pane stays
|
||||
// mounted (rename rekey, AI edit), the pane re-clones and this cleanup
|
||||
// fires with a captured generation the user never touched — emitting
|
||||
// it would push the PREVIOUS generation's content back into the entry,
|
||||
// which re-clones and re-emits forever (a microtask ping-pong that
|
||||
// pins the tab and spams the draft autosave endpoint).
|
||||
if (isDraftRun && (captured.content ?? '') === contentAtRegister) return
|
||||
// Draft runs: only emit when the user actually typed into THIS clone
|
||||
// (and its reads were already captured once). When the drafts entry is
|
||||
// rewritten externally while the pane stays mounted (rename rekey, AI
|
||||
// edit), the pane re-clones and this cleanup fires with a captured
|
||||
// generation the user never touched — emitting it would push the
|
||||
// PREVIOUS generation's content back into the entry, which re-clones
|
||||
// and re-emits forever (a microtask ping-pong that pins the tab and
|
||||
// spams the draft autosave endpoint). Each emit records inputAssets,
|
||||
// so the ping-pong still converges after one bounce for entries that
|
||||
// predate the capture.
|
||||
if (isDraftRun && capturedHadReads && (captured.content ?? '') === contentAtRegister) return
|
||||
if (!isDraftRun) {
|
||||
// Prefer the freshest fetch when it's still this script
|
||||
// (cleanup reads are untracked): after the pane's own Save +
|
||||
|
||||
@@ -531,6 +531,7 @@
|
||||
{requestRunCascadeSignal}
|
||||
{focusUploadSignal}
|
||||
draftScript={activeDraft?.script}
|
||||
draftInputsCaptured={activeDraft?.inputAssets != undefined}
|
||||
{pathPrefix}
|
||||
{onDraftPathChange}
|
||||
{workspace}
|
||||
|
||||
@@ -994,3 +994,33 @@ describe('inactive draft input lineage', () => {
|
||||
expect(r.edges.filter((e) => e.asset_path === 'main/stale')).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
describe('open saved script: live overlay vs inferred-lineage maps', () => {
|
||||
it('does not duplicate edges when the same live ref reaches both paths', () => {
|
||||
// The route page mirrors the open pane's liveBodyAssets into
|
||||
// inferredReads/WritesByPath, so the same refs arrive twice.
|
||||
const base = baseGraph({
|
||||
runnables: [{ path: 'f/x/cons', usage_kind: 'script', in_pipeline: true }]
|
||||
})
|
||||
const live = [duck('main/src', 'r'), s3('/out.parquet', 'w')]
|
||||
const r = resolveGraph(
|
||||
input({
|
||||
base,
|
||||
liveBodyAssets: { scriptPath: 'f/x/cons', assets: live },
|
||||
liveAnnotations: { scriptPath: 'f/x/cons', annotations: ann({ inPipeline: true }) },
|
||||
inferredReadsByPath: new Map([
|
||||
['f/x/cons', [{ kind: 'ducklake' as const, path: 'main/src' }]]
|
||||
]),
|
||||
inferredWritesByPath: new Map([
|
||||
['f/x/cons', [{ kind: 's3object' as const, path: '/out.parquet' }]]
|
||||
])
|
||||
})
|
||||
)
|
||||
expect(
|
||||
r.edges.filter((e) => e.asset_path === 'main/src' && e.runnable_path === 'f/x/cons')
|
||||
).toHaveLength(1)
|
||||
expect(
|
||||
r.edges.filter((e) => e.asset_path === '/out.parquet' && e.runnable_path === 'f/x/cons')
|
||||
).toHaveLength(1)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -798,7 +798,7 @@ function crossCheckSweptScripts(acc: Accumulator, input: ResolveGraphInput) {
|
||||
* changes — not just while selected.
|
||||
*/
|
||||
function overlayInferredLineage(acc: Accumulator, input: ResolveGraphInput) {
|
||||
const { base, drafts, inferredWritesByPath, inferredReadsByPath } = input
|
||||
const { drafts, inferredWritesByPath, inferredReadsByPath } = input
|
||||
const { assets, edges } = acc
|
||||
|
||||
const overlayLineage = (
|
||||
@@ -807,19 +807,21 @@ function overlayInferredLineage(acc: Accumulator, input: ResolveGraphInput) {
|
||||
) => {
|
||||
for (const [scriptPath, refs] of byPath) {
|
||||
if (drafts.has(scriptPath)) continue
|
||||
const persisted = new Set(
|
||||
base.edges
|
||||
.filter(
|
||||
(e) =>
|
||||
e.runnable_path === scriptPath &&
|
||||
e.runnable_kind === 'script' &&
|
||||
(e.access_type === access || e.access_type === 'rw')
|
||||
)
|
||||
.map((e) => `${e.asset_kind}:${e.asset_path}`)
|
||||
)
|
||||
for (const a of refs) {
|
||||
const key = `${a.kind}:${a.path}`
|
||||
if (persisted.has(key)) continue
|
||||
// Dedup against the ACCUMULATED edges, not just base: for the open
|
||||
// script these same refs may already be overlaid by
|
||||
// applyLiveBufferOverlay (which feeds the maps on the route page),
|
||||
// and a duplicate would collide on the canvas's endpoint-derived
|
||||
// edge ids.
|
||||
const hasEdge = edges.some(
|
||||
(e) =>
|
||||
e.runnable_path === scriptPath &&
|
||||
e.runnable_kind === 'script' &&
|
||||
e.asset_kind === a.kind &&
|
||||
e.asset_path === a.path &&
|
||||
(e.access_type === access || e.access_type === 'rw')
|
||||
)
|
||||
if (hasEdge) continue
|
||||
if (!assets.some((x) => x.kind === a.kind && x.path === a.path)) {
|
||||
assets.push({ kind: a.kind, path: a.path })
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user