diff --git a/backend/parsers/windmill-parser/src/asset_parser.rs b/backend/parsers/windmill-parser/src/asset_parser.rs index c2cf65309b..b782c78364 100644 --- a/backend/parsers/windmill-parser/src/asset_parser.rs +++ b/backend/parsers/windmill-parser/src/asset_parser.rs @@ -951,6 +951,12 @@ pub fn parse_pipeline_annotations(code: &str) -> PipelineAnnotations { } } + // `// macros` wins over `// pipeline`: a macro library is definition-only + // (injected into consumers; running it is a no-op), never a pipeline member. + if out.macros { + out.in_pipeline = false; + } + out } @@ -1316,6 +1322,17 @@ mod pipeline_annotation_tests { assert!(!parse_pipeline_annotations("// macros_v2\n").macros); } + #[test] + fn macros_wins_over_pipeline_membership() { + // A `// macros` library is definition-only, so `// pipeline` is ignored: + // `in_pipeline` is forced false even when both markers are present. + let out = parse_pipeline_annotations("// pipeline\n// macros\nCREATE MACRO m(a) AS a;"); + assert!(out.macros); + assert!(!out.in_pipeline); + // A plain `// pipeline` script (no macros) stays a member. + assert!(parse_pipeline_annotations("// pipeline\nSELECT 1;").in_pipeline); + } + #[test] fn use_accumulates_dedups_and_rejects_prose() { let out = parse_pipeline_annotations( diff --git a/backend/parsers/windmill-parser/tests/fixtures/pipeline_annotations.json b/backend/parsers/windmill-parser/tests/fixtures/pipeline_annotations.json index d41d05b58b..ffd40fcef2 100644 --- a/backend/parsers/windmill-parser/tests/fixtures/pipeline_annotations.json +++ b/backend/parsers/windmill-parser/tests/fixtures/pipeline_annotations.json @@ -670,6 +670,20 @@ "macros": true } }, + { + "name": "macros wins over pipeline (in_pipeline forced false)", + "code": "// pipeline\n// macros\nCREATE MACRO dbl(a) AS a * 2;", + "expected": { + "in_pipeline": false, + "asset_triggers": [], + "native_triggers": [], + "partition": null, + "freshness": null, + "tag": null, + "retry": null, + "macros": true + } + }, { "name": "macros with trailing prose is not a marker", "code": "// macros are defined below\nCREATE MACRO dbl(a) AS a * 2;", @@ -687,7 +701,7 @@ "name": "macros marker with sql comment prefix", "code": "-- macros\n-- pipeline\nCREATE MACRO dbl(a) AS a * 2;", "expected": { - "in_pipeline": true, + "in_pipeline": false, "asset_triggers": [], "native_triggers": [], "partition": null, diff --git a/cli/src/commands/pipeline/localGraph.ts b/cli/src/commands/pipeline/localGraph.ts index 14460b677a..8b05235545 100644 --- a/cli/src/commands/pipeline/localGraph.ts +++ b/cli/src/commands/pipeline/localGraph.ts @@ -457,6 +457,11 @@ export async function buildLocalPipelineGraph(args: { const folderDir = path.join(args.root, "f", folderClean); const all = await collectScripts(folderDir, args.root, args.defaultTs); + // `// macros` DuckDB libraries across the whole workspace (a pipeline may use a + // shared library outside its folder). `// macros` wins over `// pipeline`, so a + // library is never a pipeline member — matching the backend parser precedence. + const libMacros = collectMacroLibraries(args.root); + const runnables: GraphRunnable[] = []; const edges: GraphEdge[] = []; const triggers: GraphTrigger[] = []; @@ -472,7 +477,9 @@ export async function buildLocalPipelineGraph(args: { for (const s of all) { const out = await inferScriptAssets(s.content, s.language); - if (!out.in_pipeline) continue; // not a pipeline member + // Not a member: no `// pipeline` marker, OR a `// macros` library (which the + // pinned wasm still reports as `in_pipeline` — apply the precedence here). + if (!out.in_pipeline || libMacros.has(s.path)) continue; const retry = normalizeRetry(out.retry); const nativeTriggers = recoverHeaderNativeTriggers(s.content, s.language); // Carry the parsed `// tag` so previews route to the same worker the @@ -599,15 +606,6 @@ export async function buildLocalPipelineGraph(args: { } } - // `// macros` libraries + lib→consumer edges. The wasm asset parser drops the - // `// macros` / `// use` annotations and never emits a macro registry, so we - // derive both from the working tree here (see ./duckdbMacros.ts) to match the - // deployed graph, which records them at deploy. Libraries are discovered - // WORKSPACE-WIDE (not just this folder) — the deployed builder fetches the - // macro registry unfiltered so a shared library (e.g. `f/shared/stats`) is the - // provider endpoint of an in-folder consumer's edge; only consumers are - // folder-scoped. Macros are DuckDB-only. - const libMacros = collectMacroLibraries(args.root); const macroEdges = buildMacroEdges(all, libMacros, runnables); const assets = [...assetSet.entries()].map(([key, a]) => { @@ -665,13 +663,10 @@ function collectMacroLibraries(root: string): Map { return out; } -// Resolve which of this folder's pipeline scripts call the workspace's macro -// libraries (by lexical call detection + `// use` annotations), then (a) mutate -// `runnables` to add each referenced library as a node carrying its macro -// signatures, and (b) return the lib→consumer edges. Mirrors the deployed graph -// builder (`asset_graph` in windmill-api-assets): libraries come from the -// workspace-wide registry, consumers are folder-scoped, and a library node -// appears only when it is an endpoint of at least one edge (unused → not shown). +// Derive lib→consumer edges (lexical calls + `// use`) and add each referenced +// library to `runnables` as a node with its signatures. Mirrors the deployed +// `asset_graph`: libraries are workspace-wide, consumers folder-scoped, and an +// unused library (no edge) is not surfaced. function buildMacroEdges( all: LocalScript[], libMacros: Map, @@ -755,28 +750,17 @@ function buildMacroEdges( a.consumer_path.localeCompare(b.consumer_path), ); - // Tag library nodes with their macro signatures, like the deployed builder - // (which sets `macros` on any node whose path provides macros, edge or not). - // Two sources: - // • every edge provider — added as a node if it isn't already one; and - // • every library that is ALREADY a runnable (a `// pipeline` + `// macros` - // script), even with no consumers yet — so it's recognized as - // definition-only and never scheduled as a manual root. - // An unused NON-pipeline library stays absent (suppressed), matching deployed. + // A library node surfaces only when it is an edge provider (a consumer uses + // it); an unused library is not shown. Libraries are never pipeline members + // (`// macros` wins over `// pipeline`), so each is a fresh node here. const libPaths = new Set(edges.map((e) => e.lib_path)); - const existingPaths = new Set(runnables.map((r) => r.path)); - for (const lib of libMacros.keys()) { - if (existingPaths.has(lib)) libPaths.add(lib); - } for (const lib of libPaths) { const macros = (libMacros.get(lib) ?? []).map((m) => ({ name: m.name, params: m.params, is_table: m.isTable, })); - const existing = runnables.find((r) => r.path === lib); - if (existing) existing.macros = macros; - else runnables.push({ path: lib, usage_kind: "script", macros }); + runnables.push({ path: lib, usage_kind: "script", macros }); } // Force every edge's CONSUMER endpoint into the node set too, like the deployed // builder, so no edge dangles at a missing runnable. A consumer that is itself diff --git a/cli/test/pipeline_local_graph_unit.test.ts b/cli/test/pipeline_local_graph_unit.test.ts index 151d59e5ef..a0eeefe986 100644 --- a/cli/test/pipeline_local_graph_unit.test.ts +++ b/cli/test/pipeline_local_graph_unit.test.ts @@ -526,25 +526,48 @@ test("a non-pipeline DuckDB macro consumer is a display-only node, never a run s ); }); -test("an UNUSED `// pipeline` + `// macros` library is still tagged with its macros", async () => { - // A DuckDB file can be both `// pipeline` and `// macros`. The deployed builder - // tags such a node with its macros even when no consumer uses it yet, so it's - // recognized as definition-only and never scheduled as a manual root. Local - // enrichment must do the same (otherwise `run --local` would run it). +test("`// macros` wins over `// pipeline`: the library is never a pipeline member", async () => { + // A DuckDB file marked both `// pipeline` and `// macros` is treated as a + // definition-only library (mirrors the backend parser precedence). So an + // UNUSED such library is suppressed exactly like a plain `// macros` one — not + // a member node, hence never a run step. await withFolder( { "lib.duckdb.sql": `-- pipeline\n-- macros\nCREATE MACRO dbl(a) AS a * 2;\n`, "root.duckdb.sql": `-- pipeline\n-- materialize ducklake://main/out\nSELECT 1 AS v;\n`, }, async (root, folder) => { - const { graph } = await buildLocalPipelineGraph({ root, folder, defaultTs: "bun" }); - // the library node carries its macro signatures despite having no consumers + const { graph, scripts } = await buildLocalPipelineGraph({ root, folder, defaultTs: "bun" }); + // unused library → suppressed (not a node), and never a previewable member + expect(graph.runnables.map((r) => r.path)).toEqual(["f/mypipe/root"]); + expect(scripts.map((s) => s.path)).toEqual(["f/mypipe/root"]); + expect(graph.macro_edges).toBeUndefined(); + }, + ); +}); + +test("a USED `// pipeline` + `// macros` library appears as a library node (not a member)", async () => { + await withFolder( + { + "lib.duckdb.sql": `-- pipeline\n-- macros\nCREATE MACRO dbl(a) AS a * 2;\n`, + "root.duckdb.sql": `-- pipeline\n-- on datatable://main/t\nSELECT dbl(x) FROM main.t;\n`, + }, + async (root, folder) => { + const { graph, scripts } = await buildLocalPipelineGraph({ root, folder, defaultTs: "bun" }); + // the library surfaces as a node with its signatures (it's used) … expect(graph.runnables.find((r) => r.path === "f/mypipe/lib")?.macros).toEqual([ { name: "dbl", params: "a", is_table: false }, ]); - // no consumer → no macro edges, but the node is still marked (macros.length - // > 0 is what `pipeline run` uses to exclude it from the run selection) - expect(graph.macro_edges).toBeUndefined(); + // … but is NOT a pipeline member: absent from the previewable `scripts` set + expect(scripts.map((s) => s.path)).toEqual(["f/mypipe/root"]); + expect(graph.macro_edges).toEqual([ + { + lib_path: "f/mypipe/lib", + consumer_path: "f/mypipe/root", + macro_names: ["dbl"], + via_use: false, + }, + ]); }, ); }); diff --git a/frontend/src/lib/components/assets/AssetGraph/parsePipelineAnnotations.test.ts b/frontend/src/lib/components/assets/AssetGraph/parsePipelineAnnotations.test.ts index e9e23525ec..91dba1c658 100644 --- a/frontend/src/lib/components/assets/AssetGraph/parsePipelineAnnotations.test.ts +++ b/frontend/src/lib/components/assets/AssetGraph/parsePipelineAnnotations.test.ts @@ -108,6 +108,12 @@ describe('parsePipelineAnnotations: macros + use', () => { expect(parsePipelineAnnotations('-- macros \nSELECT 1;').macros).toBe(true) }) + it('macros wins over pipeline — a library is never a pipeline member', () => { + const out = parsePipelineAnnotations('// pipeline\n// macros\nCREATE MACRO m(a) AS a;') + expect(out.macros).toBe(true) + expect(out.inPipeline).toBe(false) + }) + it('use accumulates in order and dedups', () => { const out = parsePipelineAnnotations( '// use f/lib/stats\n// use f/lib/dates\n// use f/lib/stats\n' diff --git a/frontend/src/lib/components/assets/AssetGraph/parsePipelineAnnotations.ts b/frontend/src/lib/components/assets/AssetGraph/parsePipelineAnnotations.ts index 4d355803e6..8087d81577 100644 --- a/frontend/src/lib/components/assets/AssetGraph/parsePipelineAnnotations.ts +++ b/frontend/src/lib/components/assets/AssetGraph/parsePipelineAnnotations.ts @@ -685,5 +685,9 @@ export function parsePipelineAnnotations(code: string): PipelineAnnotations { } } + // `// macros` wins over `// pipeline`: a macro library is definition-only, + // never a pipeline member. Mirrors the Rust parse_pipeline_annotations. + if (out.macros) out.inPipeline = false + return out }