Skip to content

Commit 73fbdec

Browse files
committed
Move outputs to gate-on-binding model
- Binding gains `gate: GateAtom[]` carrying root-to-leaf wrapper atoms; GateAtom adds an `iter` kind for repeat-driven list iteration. - ResolvedOutput slimmed to pure template data; per-output gating is derived at codegen via `outputGate(scopeGate, output, bindings)`, which lives in the bindings layer and combines the scope binding's gate, each ref binding's gate, and a type-derived atom for nullable or iterable ref types. - Outputs attach to declaring struct sequences only; drop LCA hosting and parent-map machinery in the Boutiques parser. The source's `output-files[].optional` hint is re-derived rather than stored. - Solver forces a struct binding on output-carrying sequences so scopes are always real BindingIds. - SolveResult slimmed to `{ bindings, resolve }`; `resolveOutputs` is a separate top-level phase returning `{ scopes, diagnostics }`. - 487 tests pass; ~150 lines deleted across solver, parser, and the Boutiques backend.
1 parent dc843c6 commit 73fbdec

34 files changed

Lines changed: 872 additions & 1336 deletions

packages/core/src/backend/boutiques/boutiques.test.ts

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, expect, it } from "vitest";
2-
import { solve } from "../../solver/solver.js";
2+
import { resolveOutputs, solve } from "../../solver/index.js";
33
import { defaultPipeline } from "../../ir/index.js";
44
import { BoutiquesParser } from "../../frontend/boutiques/parser.js";
55
import { ArgdumpParser } from "../../frontend/argdump/parser.js";
@@ -12,7 +12,8 @@ function emitFor(descriptor: Record<string, unknown>): Record<string, unknown> {
1212
const { expr, meta } = parser.parse(JSON.stringify(descriptor));
1313
const optimized = defaultPipeline.apply(expr).expr;
1414
const solveResult = solve(optimized);
15-
const ctx = createContext(optimized, solveResult, { app: meta });
15+
const outputs = resolveOutputs(optimized, solveResult);
16+
const ctx = createContext(optimized, solveResult, outputs, { app: meta });
1617
const { descriptor: bt } = generateBoutiques(ctx);
1718
return bt as Record<string, unknown>;
1819
}
@@ -287,9 +288,7 @@ describe("Boutiques subcommands", () => {
287288
type: {
288289
id: "sub",
289290
"command-line": "--name [NAME]",
290-
inputs: [
291-
{ id: "name", "value-key": "[NAME]", type: "String" },
292-
],
291+
inputs: [{ id: "name", "value-key": "[NAME]", type: "String" }],
293292
},
294293
},
295294
],
@@ -325,9 +324,7 @@ describe("Boutiques subcommands", () => {
325324
id: "mode_b",
326325
name: "Mode B",
327326
"command-line": "--mode b [NUM]",
328-
inputs: [
329-
{ id: "num", "value-key": "[NUM]", type: "Number", integer: true },
330-
],
327+
inputs: [{ id: "num", "value-key": "[NUM]", type: "Number", integer: true }],
331328
},
332329
],
333330
},
@@ -415,7 +412,8 @@ describe("BoutiquesBackend", () => {
415412
);
416413
const optimized = defaultPipeline.apply(expr).expr;
417414
const solveResult = solve(optimized);
418-
const ctx = createContext(optimized, solveResult, { app: meta });
415+
const outputs = resolveOutputs(optimized, solveResult);
416+
const ctx = createContext(optimized, solveResult, outputs, { app: meta });
419417

420418
const backend = new BoutiquesBackend();
421419
const result = backend.emit(ctx);
@@ -448,7 +446,8 @@ describe("BoutiquesBackend", () => {
448446
);
449447
const optimized = defaultPipeline.apply(expr).expr;
450448
const solveResult = solve(optimized);
451-
const ctx = createContext(optimized, solveResult, { app: meta });
449+
const outputs = resolveOutputs(optimized, solveResult);
450+
const ctx = createContext(optimized, solveResult, outputs, { app: meta });
452451

453452
const backend = new BoutiquesBackend();
454453
const result = backend.emit(ctx);
@@ -470,7 +469,8 @@ describe("argdump -> Boutiques validity", () => {
470469
const { expr, meta } = argdumpParser.parse(JSON.stringify(dump));
471470
const optimized = defaultPipeline.apply(expr).expr;
472471
const solveResult = solve(optimized);
473-
const ctx = createContext(optimized, solveResult, { app: meta });
472+
const outputs = resolveOutputs(optimized, solveResult);
473+
const ctx = createContext(optimized, solveResult, outputs, { app: meta });
474474
const { descriptor: bt } = generateBoutiques(ctx);
475475
return bt as Record<string, unknown>;
476476
}
@@ -630,9 +630,7 @@ describe("argdump -> Boutiques validity", () => {
630630
action_type: "store_true",
631631
},
632632
],
633-
mutually_exclusive_groups: [
634-
{ required: false, actions: ["input_file", "no_input"] },
635-
],
633+
mutually_exclusive_groups: [{ required: false, actions: ["input_file", "no_input"] }],
636634
});
637635
const inputs = bt.inputs as Record<string, unknown>[];
638636
const parent = inputs.find((i) => i.id === "input_file_or_no_input");
@@ -665,9 +663,7 @@ describe("argdump -> Boutiques validity", () => {
665663
action_type: "store_true",
666664
},
667665
],
668-
mutually_exclusive_groups: [
669-
{ required: false, actions: ["input_file", "no_input"] },
670-
],
666+
mutually_exclusive_groups: [{ required: false, actions: ["input_file", "no_input"] }],
671667
});
672668
const inputs = bt.inputs as Record<string, unknown>[];
673669
const parent = inputs.find((i) => i.id === "input_file_or_no_input");
@@ -865,18 +861,19 @@ describe("Boutiques output-files emission", () => {
865861
expect(files[0]!.description).toBe("The output of the tool");
866862
});
867863

868-
it("emits output-files[].optional from the parser-side optional flag", () => {
864+
it("does not emit `optional: true` when no ref is gated (the source hint is dropped)", () => {
865+
// The Boutiques source `optional: true` is a tool-author hint we re-derive
866+
// structurally - so an output whose refs all point to required inputs
867+
// emits without the flag, regardless of the hint.
869868
const bt = emitFor(
870869
minimalDescriptor({
871870
"command-line": "tool [INPUT_FILE]",
872871
inputs: [{ id: "input_file", name: "In", type: "File", "value-key": "[INPUT_FILE]" }],
873-
"output-files": [
874-
{ id: "maybe", "path-template": "[INPUT_FILE].extra", optional: true },
875-
],
872+
"output-files": [{ id: "maybe", "path-template": "[INPUT_FILE].extra", optional: true }],
876873
}),
877874
);
878875
const files = bt["output-files"] as Record<string, unknown>[];
879-
expect(files[0]!.optional).toBe(true);
876+
expect(files[0]!.optional).toBeUndefined();
880877
});
881878

882879
it("emits an output hosted inside a subcommand arm on that arm's descriptor", () => {
@@ -958,12 +955,15 @@ describe("Boutiques output-files round-trip", () => {
958955
expect(reFiles["path-template"]).toBe(`${inputKey}.out`);
959956
});
960957

961-
it("round-trips an optional output flag", () => {
958+
it("round-trips an output optional flag derived from an optional ref", () => {
959+
// The optional input drives the structural optionality of the output.
962960
const descriptor = {
963961
name: "tool",
964962
"command-line": "tool [INPUT_FILE]",
965-
inputs: [{ id: "input_file", name: "In", type: "File", "value-key": "[INPUT_FILE]" }],
966-
"output-files": [{ id: "maybe", "path-template": "[INPUT_FILE].extra", optional: true }],
963+
inputs: [
964+
{ id: "input_file", name: "In", type: "File", "value-key": "[INPUT_FILE]", optional: true },
965+
],
966+
"output-files": [{ id: "maybe", "path-template": "[INPUT_FILE].extra" }],
967967
};
968968
const emitted = roundTrip(descriptor);
969969
const files = emitted["output-files"] as Record<string, unknown>[];

packages/core/src/backend/boutiques/boutiques.ts

Lines changed: 47 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import type {
33
BindingId,
44
BoundType,
55
BoundVariant,
6+
GateAtom,
7+
OutputScope,
68
ResolvedOutput,
79
} from "../../bindings/index.js";
810
import type { Expr, ScalarKind } from "../../ir/index.js";
@@ -12,6 +14,7 @@ import type { Backend, EmitResult, EmitWarning } from "../backend.js";
1214
import { collectFieldInfo } from "../collect-field-info.js";
1315
import { findDoc } from "../find-doc.js";
1416
import { findStructNode } from "../find-struct-node.js";
17+
import { outputGate } from "../resolve-output-tokens.js";
1518
import { resolveFieldBinding } from "../resolve-field-binding.js";
1619
import { Scope } from "../scope.js";
1720
import { screamingSnakeCase } from "../string-case.js";
@@ -80,77 +83,23 @@ interface PeeledInput {
8083

8184
class BoutiquesEmitter {
8285
private warnings: EmitWarning[] = [];
83-
// Parent map over ctx.expr - used to find each output host's owning scope.
84-
private parents = new Map<Expr, Expr | null>();
85-
// Owning descriptor scope -> outputs hosted somewhere in that scope. The
86-
// scope key is either the root expr or an `alternative` arm node; outputs
87-
// emit on the descriptor built from that node.
88-
private outputsByScope = new Map<Expr, ResolvedOutput[]>();
86+
// Scope binding id -> outputs declared on that struct. The solver forces a
87+
// binding on every output-carrying sequence, so this is a one-liner.
88+
private outputsByScope = new Map<BindingId, OutputScope>();
8989

90-
constructor(private ctx: CodegenContext) {}
90+
constructor(private ctx: CodegenContext) {
91+
for (const scope of ctx.outputScopes) this.outputsByScope.set(scope.scope, scope);
92+
}
9193

9294
private warn(message: string): void {
9395
this.warnings.push({ message });
9496
}
9597

9698
emit(): { descriptor: BtDescriptor; warnings: EmitWarning[] } {
97-
this.indexOutputs();
9899
const descriptor = this.buildRootDescriptor();
99100
return { descriptor, warnings: this.warnings };
100101
}
101102

102-
// Build the parent map and the per-scope output groups in one shot. The
103-
// solver already exposes each output's host node (the IR node carrying it
104-
// in NodeMeta.outputs); we just bucket outputs by owning descriptor scope.
105-
private indexOutputs(): void {
106-
this.buildParentMap(this.ctx.expr, null);
107-
for (const output of this.ctx.outputs) {
108-
const host = this.ctx.outputHosts.get(output);
109-
if (!host) {
110-
// Should not happen - the solver populates outputHosts for every
111-
// resolved output - but degrade gracefully if it does.
112-
this.warn(`Output '${output.name}' has no host node - skipping`);
113-
continue;
114-
}
115-
const scope = this.findOwningScope(host);
116-
let bucket = this.outputsByScope.get(scope);
117-
if (!bucket) {
118-
bucket = [];
119-
this.outputsByScope.set(scope, bucket);
120-
}
121-
bucket.push(output);
122-
}
123-
}
124-
125-
private buildParentMap(node: Expr, parent: Expr | null): void {
126-
this.parents.set(node, parent);
127-
switch (node.kind) {
128-
case "sequence":
129-
for (const child of node.attrs.nodes) this.buildParentMap(child, node);
130-
break;
131-
case "optional":
132-
case "repeat":
133-
this.buildParentMap(node.attrs.node, node);
134-
break;
135-
case "alternative":
136-
for (const alt of node.attrs.alts) this.buildParentMap(alt, node);
137-
break;
138-
}
139-
}
140-
141-
// Walk from a host node toward the root. The first ancestor whose parent is
142-
// an `alternative` is the owning arm; otherwise the root expr owns it.
143-
private findOwningScope(host: Expr): Expr {
144-
let node: Expr | null = host;
145-
while (node) {
146-
const parent: Expr | null = this.parents.get(node) ?? null;
147-
if (parent === null) return this.ctx.expr;
148-
if (parent.kind === "alternative") return node;
149-
node = parent;
150-
}
151-
return this.ctx.expr;
152-
}
153-
154103
private buildRootDescriptor(): BtDescriptor {
155104
const bt: BtDescriptor = { "schema-version": "0.5+styx" };
156105

@@ -409,10 +358,7 @@ class BoutiquesEmitter {
409358
// Try to resolve to a field binding
410359
const match = resolveFieldBinding(child, this.ctx, structType);
411360
if (!match) {
412-
// Unbound node - emit as literal text if possible
413-
if (child.kind === "literal") {
414-
commandParts.push(child.attrs.str);
415-
}
361+
// Unbound non-literal node - no command-line text we can emit.
416362
continue;
417363
}
418364

@@ -437,11 +383,22 @@ class BoutiquesEmitter {
437383
if (flagStr) peeled.flag = flagStr;
438384
}
439385

440-
const input = this.buildInput(binding, id, fieldType, valueKeyStr, peeled, fieldInfo, wrapperNode);
386+
const input = this.buildInput(
387+
binding,
388+
id,
389+
fieldType,
390+
valueKeyStr,
391+
peeled,
392+
fieldInfo,
393+
wrapperNode,
394+
);
441395

442396
// Add flag to command line if present, then value-key
443397
if (peeled.flag) {
444-
if (fieldType.kind === "bool" || (fieldType.kind === "optional" && this.isBool(fieldType))) {
398+
if (
399+
fieldType.kind === "bool" ||
400+
(fieldType.kind === "optional" && this.isBool(fieldType))
401+
) {
445402
// Bool flags: the value-key IS the flag
446403
commandParts.push(valueKeyStr);
447404
} else {
@@ -460,29 +417,32 @@ class BoutiquesEmitter {
460417
this.emitOutputFiles(bt, expr, valueKeyByBinding, idScope);
461418
}
462419

463-
// Emit `output-files` entries owned by this descriptor scope. Refs in
464-
// `path-template` are resolved against `valueKeys` (binding id ->
465-
// bracketed value-key string assigned to its input here); refs that point
466-
// to bindings outside the scope are dropped with a warning. Output ids
467-
// share the input id scope so they cannot collide.
420+
// Emit `output-files` entries declared on this struct binding. `optional`
421+
// and `list` are derived from the unified gate (scope binding's gate plus
422+
// each ref's binding gate plus type-derived atoms). Refs that point to
423+
// bindings outside the scope are dropped with a warning; output ids share
424+
// the input id scope so they cannot collide.
468425
private emitOutputFiles(
469426
bt: BtDescriptor,
470-
scope: Expr,
427+
scopeNode: Expr,
471428
valueKeys: Map<BindingId, string>,
472429
idScope: Scope,
473430
): void {
474-
const outputs = this.outputsByScope.get(scope);
475-
if (!outputs || outputs.length === 0) return;
431+
const scopeBinding = this.ctx.resolve(scopeNode);
432+
if (!scopeBinding) return;
433+
const scope = this.outputsByScope.get(scopeBinding.id);
434+
if (!scope || scope.outputs.length === 0) return;
476435

477436
const files: BtOutputFile[] = [];
478-
for (const output of outputs) {
479-
const file = this.buildOutputFile(output, valueKeys, idScope);
437+
for (const output of scope.outputs) {
438+
const file = this.buildOutputFile(scopeBinding.gate, output, valueKeys, idScope);
480439
if (file) files.push(file);
481440
}
482441
if (files.length > 0) bt["output-files"] = files;
483442
}
484443

485444
private buildOutputFile(
445+
scopeGate: GateAtom[],
486446
output: ResolvedOutput,
487447
valueKeys: Map<BindingId, string>,
488448
idScope: Scope,
@@ -515,12 +475,16 @@ class BoutiquesEmitter {
515475
// output entirely. (A literal-only template stays.)
516476
if (droppedRef && template === "") return null;
517477

478+
const gate = outputGate(scopeGate, output, this.ctx.bindings);
479+
const isOptional = gate.some((a) => a.kind === "present" || a.kind === "variant");
480+
const isList = gate.some((a) => a.kind === "iter");
481+
518482
const id = idScope.add(this.sanitizeId(output.name));
519483
const file: BtOutputFile = { id, "path-template": template };
520484
if (output.doc?.title) file.name = output.doc.title;
521485
if (output.doc?.description) file.description = output.doc.description;
522-
if (output.optional) file.optional = true;
523-
if (output.listScope.length > 0) file.list = true;
486+
if (isOptional) file.optional = true;
487+
if (isList) file.list = true;
524488
if (stripExtensions) file["path-template-stripped-extensions"] = stripExtensions;
525489
return file;
526490
}
@@ -658,11 +622,7 @@ class BoutiquesEmitter {
658622
switch (node.kind) {
659623
case "optional":
660624
result.isOptional = true;
661-
this.peelNodeInner(
662-
node.attrs.node,
663-
type.kind === "optional" ? type.inner : type,
664-
result,
665-
);
625+
this.peelNodeInner(node.attrs.node, type.kind === "optional" ? type.inner : type, result);
666626
break;
667627

668628
case "repeat":
@@ -674,11 +634,7 @@ class BoutiquesEmitter {
674634
if (node.attrs.join !== undefined) result.listSeparator = node.attrs.join;
675635
if (node.attrs.countMin !== undefined) result.minListEntries = node.attrs.countMin;
676636
if (node.attrs.countMax !== undefined) result.maxListEntries = node.attrs.countMax;
677-
this.peelNodeInner(
678-
node.attrs.node,
679-
type.kind === "list" ? type.item : type,
680-
result,
681-
);
637+
this.peelNodeInner(node.attrs.node, type.kind === "list" ? type.item : type, result);
682638
break;
683639

684640
case "sequence": {
@@ -843,9 +799,7 @@ class BoutiquesEmitter {
843799
}
844800

845801
// All-struct union -> SubCommandUnion
846-
const allStruct = type.variants.every(
847-
(v: BoundVariant) => v.type.kind === "struct",
848-
);
802+
const allStruct = type.variants.every((v: BoundVariant) => v.type.kind === "struct");
849803
if (allStruct) {
850804
return { type: this.buildSubCommandUnion(type, node) };
851805
}
@@ -854,10 +808,7 @@ class BoutiquesEmitter {
854808
return { type: this.buildMixedUnionAsSubCommands(type, node) };
855809
}
856810

857-
private buildSubCommand(
858-
type: Extract<BoundType, { kind: "struct" }>,
859-
node: Expr,
860-
): BtDescriptor {
811+
private buildSubCommand(type: Extract<BoundType, { kind: "struct" }>, node: Expr): BtDescriptor {
861812
const bt: BtDescriptor = {};
862813
const structNode = findStructNode(node, this.ctx, type);
863814
if (structNode) {
@@ -999,9 +950,7 @@ class BoutiquesEmitter {
999950
// Bounded count -> String + enumerated value-choices.
1000951
// Unbounded count -> SubCommand + list:true (no list-separator: each
1001952
// occurrence must be a separate argv element for argparse to count it).
1002-
private mapCount(
1003-
node: Expr,
1004-
): {
953+
private mapCount(node: Expr): {
1005954
type: string | BtDescriptor;
1006955
valueChoices?: string[];
1007956
list?: boolean;

0 commit comments

Comments
 (0)