From 237aaf9120cfa830d666a8242243f7de311ff00c Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Tue, 7 Jul 2026 18:34:28 +0000 Subject: [PATCH] fix(pipelines): dedupe inferred-lineage overlay against accumulated edges; first draft teardown still captures reads Co-Authored-By: Claude Fable 5 --- .../AssetGraph/AssetGraphDetailsPane.svelte | 35 +++++++++++++------ .../AssetGraph/PipelineGraphEditor.svelte | 1 + .../assets/AssetGraph/resolveGraph.test.ts | 30 ++++++++++++++++ .../assets/AssetGraph/resolveGraph.ts | 28 ++++++++------- 4 files changed, 70 insertions(+), 24 deletions(-) diff --git a/frontend/src/lib/components/assets/AssetGraph/AssetGraphDetailsPane.svelte b/frontend/src/lib/components/assets/AssetGraph/AssetGraphDetailsPane.svelte index d4dc1a8943..461a178ce2 100644 --- a/frontend/src/lib/components/assets/AssetGraph/AssetGraphDetailsPane.svelte +++ b/frontend/src/lib/components/assets/AssetGraph/AssetGraphDetailsPane.svelte @@ -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 + diff --git a/frontend/src/lib/components/assets/AssetGraph/PipelineGraphEditor.svelte b/frontend/src/lib/components/assets/AssetGraph/PipelineGraphEditor.svelte index 37b79852e5..88d2c7c512 100644 --- a/frontend/src/lib/components/assets/AssetGraph/PipelineGraphEditor.svelte +++ b/frontend/src/lib/components/assets/AssetGraph/PipelineGraphEditor.svelte @@ -531,6 +531,7 @@ {requestRunCascadeSignal} {focusUploadSignal} draftScript={activeDraft?.script} + draftInputsCaptured={activeDraft?.inputAssets != undefined} {pathPrefix} {onDraftPathChange} {workspace} diff --git a/frontend/src/lib/components/assets/AssetGraph/resolveGraph.test.ts b/frontend/src/lib/components/assets/AssetGraph/resolveGraph.test.ts index 5629da692b..e35054beab 100644 --- a/frontend/src/lib/components/assets/AssetGraph/resolveGraph.test.ts +++ b/frontend/src/lib/components/assets/AssetGraph/resolveGraph.test.ts @@ -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) + }) +}) diff --git a/frontend/src/lib/components/assets/AssetGraph/resolveGraph.ts b/frontend/src/lib/components/assets/AssetGraph/resolveGraph.ts index 196d4f89e6..8ee0ca53d7 100644 --- a/frontend/src/lib/components/assets/AssetGraph/resolveGraph.ts +++ b/frontend/src/lib/components/assets/AssetGraph/resolveGraph.ts @@ -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 }) }