fix(pipelines): // macros takes precedence over // pipeline (a library is never a member)

A macro library is definition-only — its macros are injected into consumers and
running it is a no-op — so marking it `// pipeline` is meaningless and only
produced a confusing state (an unused pipeline macro library appearing as a
manual root). Make `// macros` win: parse_pipeline_annotations forces in_pipeline
false when macros is set. Mirrored in all three parsers that must agree — the Rust
canonical parser (drives deploy membership), the frontend TS parser (live graph),
and the CLI local graph (pinned wasm still reports in_pipeline, so precedence is
applied when skipping members). Shared parity fixture + unit tests on each side.
This commit is contained in:
Ruben Fiszel
2026-07-05 23:04:50 +00:00
parent 84d6d19df5
commit b398b690da
6 changed files with 91 additions and 43 deletions
@@ -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 out
} }
@@ -1316,6 +1322,17 @@ mod pipeline_annotation_tests {
assert!(!parse_pipeline_annotations("// macros_v2\n").macros); 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] #[test]
fn use_accumulates_dedups_and_rejects_prose() { fn use_accumulates_dedups_and_rejects_prose() {
let out = parse_pipeline_annotations( let out = parse_pipeline_annotations(
@@ -670,6 +670,20 @@
"macros": true "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", "name": "macros with trailing prose is not a marker",
"code": "// macros are defined below\nCREATE MACRO dbl(a) AS a * 2;", "code": "// macros are defined below\nCREATE MACRO dbl(a) AS a * 2;",
@@ -687,7 +701,7 @@
"name": "macros marker with sql comment prefix", "name": "macros marker with sql comment prefix",
"code": "-- macros\n-- pipeline\nCREATE MACRO dbl(a) AS a * 2;", "code": "-- macros\n-- pipeline\nCREATE MACRO dbl(a) AS a * 2;",
"expected": { "expected": {
"in_pipeline": true, "in_pipeline": false,
"asset_triggers": [], "asset_triggers": [],
"native_triggers": [], "native_triggers": [],
"partition": null, "partition": null,
+16 -32
View File
@@ -457,6 +457,11 @@ export async function buildLocalPipelineGraph(args: {
const folderDir = path.join(args.root, "f", folderClean); const folderDir = path.join(args.root, "f", folderClean);
const all = await collectScripts(folderDir, args.root, args.defaultTs); 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 runnables: GraphRunnable[] = [];
const edges: GraphEdge[] = []; const edges: GraphEdge[] = [];
const triggers: GraphTrigger[] = []; const triggers: GraphTrigger[] = [];
@@ -472,7 +477,9 @@ export async function buildLocalPipelineGraph(args: {
for (const s of all) { for (const s of all) {
const out = await inferScriptAssets(s.content, s.language); 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 retry = normalizeRetry(out.retry);
const nativeTriggers = recoverHeaderNativeTriggers(s.content, s.language); const nativeTriggers = recoverHeaderNativeTriggers(s.content, s.language);
// Carry the parsed `// tag` so previews route to the same worker the // 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 macroEdges = buildMacroEdges(all, libMacros, runnables);
const assets = [...assetSet.entries()].map(([key, a]) => { const assets = [...assetSet.entries()].map(([key, a]) => {
@@ -665,13 +663,10 @@ function collectMacroLibraries(root: string): Map<string, ParsedMacro[]> {
return out; return out;
} }
// Resolve which of this folder's pipeline scripts call the workspace's macro // Derive lib→consumer edges (lexical calls + `// use`) and add each referenced
// libraries (by lexical call detection + `// use` annotations), then (a) mutate // library to `runnables` as a node with its signatures. Mirrors the deployed
// `runnables` to add each referenced library as a node carrying its macro // `asset_graph`: libraries are workspace-wide, consumers folder-scoped, and an
// signatures, and (b) return the lib→consumer edges. Mirrors the deployed graph // unused library (no edge) is not surfaced.
// 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).
function buildMacroEdges( function buildMacroEdges(
all: LocalScript[], all: LocalScript[],
libMacros: Map<string, ParsedMacro[]>, libMacros: Map<string, ParsedMacro[]>,
@@ -755,28 +750,17 @@ function buildMacroEdges(
a.consumer_path.localeCompare(b.consumer_path), a.consumer_path.localeCompare(b.consumer_path),
); );
// Tag library nodes with their macro signatures, like the deployed builder // A library node surfaces only when it is an edge provider (a consumer uses
// (which sets `macros` on any node whose path provides macros, edge or not). // it); an unused library is not shown. Libraries are never pipeline members
// Two sources: // (`// macros` wins over `// pipeline`), so each is a fresh node here.
// • 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.
const libPaths = new Set<string>(edges.map((e) => e.lib_path)); const libPaths = new Set<string>(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) { for (const lib of libPaths) {
const macros = (libMacros.get(lib) ?? []).map((m) => ({ const macros = (libMacros.get(lib) ?? []).map((m) => ({
name: m.name, name: m.name,
params: m.params, params: m.params,
is_table: m.isTable, is_table: m.isTable,
})); }));
const existing = runnables.find((r) => r.path === lib); runnables.push({ path: lib, usage_kind: "script", macros });
if (existing) existing.macros = macros;
else runnables.push({ path: lib, usage_kind: "script", macros });
} }
// Force every edge's CONSUMER endpoint into the node set too, like the deployed // 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 // builder, so no edge dangles at a missing runnable. A consumer that is itself
+33 -10
View File
@@ -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 () => { test("`// macros` wins over `// pipeline`: the library is never a pipeline member", async () => {
// A DuckDB file can be both `// pipeline` and `// macros`. The deployed builder // A DuckDB file marked both `// pipeline` and `// macros` is treated as a
// tags such a node with its macros even when no consumer uses it yet, so it's // definition-only library (mirrors the backend parser precedence). So an
// recognized as definition-only and never scheduled as a manual root. Local // UNUSED such library is suppressed exactly like a plain `// macros` one — not
// enrichment must do the same (otherwise `run --local` would run it). // a member node, hence never a run step.
await withFolder( await withFolder(
{ {
"lib.duckdb.sql": `-- pipeline\n-- macros\nCREATE MACRO dbl(a) AS a * 2;\n`, "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`, "root.duckdb.sql": `-- pipeline\n-- materialize ducklake://main/out\nSELECT 1 AS v;\n`,
}, },
async (root, folder) => { async (root, folder) => {
const { graph } = await buildLocalPipelineGraph({ root, folder, defaultTs: "bun" }); const { graph, scripts } = await buildLocalPipelineGraph({ root, folder, defaultTs: "bun" });
// the library node carries its macro signatures despite having no consumers // 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([ expect(graph.runnables.find((r) => r.path === "f/mypipe/lib")?.macros).toEqual([
{ name: "dbl", params: "a", is_table: false }, { name: "dbl", params: "a", is_table: false },
]); ]);
// no consumer → no macro edges, but the node is still marked (macros.length // … but is NOT a pipeline member: absent from the previewable `scripts` set
// > 0 is what `pipeline run` uses to exclude it from the run selection) expect(scripts.map((s) => s.path)).toEqual(["f/mypipe/root"]);
expect(graph.macro_edges).toBeUndefined(); expect(graph.macro_edges).toEqual([
{
lib_path: "f/mypipe/lib",
consumer_path: "f/mypipe/root",
macro_names: ["dbl"],
via_use: false,
},
]);
}, },
); );
}); });
@@ -108,6 +108,12 @@ describe('parsePipelineAnnotations: macros + use', () => {
expect(parsePipelineAnnotations('-- macros \nSELECT 1;').macros).toBe(true) 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', () => { it('use accumulates in order and dedups', () => {
const out = parsePipelineAnnotations( const out = parsePipelineAnnotations(
'// use f/lib/stats\n// use f/lib/dates\n// use f/lib/stats\n' '// use f/lib/stats\n// use f/lib/dates\n// use f/lib/stats\n'
@@ -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 return out
} }