Skip to content

Commit cc78639

Browse files
nx10nx10
authored
refactor: dead-code removal and de-duplication (#64)
* refactor(backend): remove the dead output-emit "plan" API `planOutput`, `planScope`, `isGated`, `isIterated`, `compactTokens`, and `OutputEmitPlan` were exported (and tested) but no backend consumed them - every real consumer uses only the re-exported `outputGate`. Drop the unused cluster and its tests; keep `outputGate` as the single entry point. Not part of the hub public-API surface, so no consumer is affected. * refactor(boutiques): collapse two identical union-descriptor builders `buildSubCommandUnion` and `buildMixedUnionAsSubCommands` were byte-for-byte identical, so the `allStruct` split in `mapUnion` chose between two copies of the same logic. Collapse them into one `buildUnionSubCommands` and drop the dead branch distinction. * refactor(backend): share collectDefaults/rootFieldDefault across backends The field-default collection and root-field-default lookup were copy-pasted four times (each backend's arg-builder and outputs-emit, mirrored across Python and TypeScript), differing only in the literal renderer. Extract them into `field-defaults.ts` parameterized by a `renderLiteral` hook. Generated output is byte-identical (verified by the snapshot/execution tests and the codegen gate). * refactor(backend): inline the vestigial `emitOutputs` flag Both codegen backends set `const emitOutputs = true` and then guarded every outputs-related emit with it, and passed `true` to `emitImports`. Every tool emits an Outputs object, so the flag was dead flexibility. Inline it (guards become unconditional, `emitImports` drops the parameter). Output order preserved. * refactor: drop unreachable branches and an unused parameter - `isBooleanLiteralPair`: the `"0"`/`"1"` string comparisons are unreachable - `literalFromNode` canonicalizes clean-int literals to the numbers 0/1, so only "false"/"true" ever arrive as strings. - argdump `applyMutualExclusion`: the `actionsByDest` map was built, threaded in, and never read. Remove the map, the population, the parameter, and the argument.
1 parent f89ac36 commit cc78639

15 files changed

Lines changed: 97 additions & 375 deletions

File tree

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

Lines changed: 7 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -844,14 +844,9 @@ class BoutiquesEmitter {
844844
};
845845
}
846846

847-
// All-struct union -> SubCommandUnion
848-
const allStruct = type.variants.every((v: BoundVariant) => v.type.kind === "struct");
849-
if (allStruct) {
850-
return { type: this.buildSubCommandUnion(type, node) };
851-
}
852-
853-
// Mixed union -> wrap each variant as a SubCommand descriptor
854-
return { type: this.buildMixedUnionAsSubCommands(type, node) };
847+
// Any other union (all-struct or mixed) -> one SubCommand descriptor per
848+
// variant: struct arms serialize recursively, non-struct arms are wrapped.
849+
return { type: this.buildUnionSubCommands(type, node) };
855850
}
856851

857852
private buildSubCommand(type: Extract<BoundType, { kind: "struct" }>, node: Expr): BtDescriptor {
@@ -868,29 +863,10 @@ class BoutiquesEmitter {
868863
return bt;
869864
}
870865

871-
private buildSubCommandUnion(
872-
type: Extract<BoundType, { kind: "union" }>,
873-
node: Expr,
874-
): BtDescriptor[] {
875-
const alts = node.kind === "alternative" ? node.attrs.alts : [node];
876-
877-
return type.variants.map((variant: BoundVariant, i: number) => {
878-
const altNode = alts[i] ?? node;
879-
if (variant.type.kind === "struct") {
880-
const bt = this.buildSubCommand(variant.type, altNode);
881-
// The variant has its own name; the wrapperNode's name is the
882-
// parent (mutex group) name and would otherwise leak down.
883-
if (variant.name) {
884-
bt.name = variant.name;
885-
bt.id = this.sanitizeId(variant.name);
886-
}
887-
return bt;
888-
}
889-
return this.wrapAsDescriptor(variant, altNode);
890-
});
891-
}
892-
893-
private buildMixedUnionAsSubCommands(
866+
// One SubCommand descriptor per union variant. Handles both all-struct and
867+
// mixed unions: struct arms serialize recursively, non-struct arms are wrapped
868+
// as a trivial single-input descriptor.
869+
private buildUnionSubCommands(
894870
type: Extract<BoundType, { kind: "union" }>,
895871
node: Expr,
896872
): BtDescriptor[] {

‎packages/core/src/backend/field-defaults.ts‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import type { Binding, BoundType } from "../bindings/index.js";
2+
import type { CodegenContext } from "../manifest/index.js";
3+
import { collectFieldInfo } from "./collect-field-info.js";
4+
5+
/** Renders a default value as a target-language literal. */
6+
export type RenderLiteral = (value: string | number | boolean) => string;
7+
8+
/**
9+
* Build the field-name -> rendered-default map for a struct root (else empty).
10+
* Includes only non-optional defaulted fields (optional fields are
11+
* presence-guarded; their default comes from the factory's kwarg signature).
12+
*
13+
* `rootType` defaults to the resolved root type; callers that already have it
14+
* (the arg-builders) pass it to avoid re-resolving.
15+
*/
16+
export function collectDefaults(
17+
ctx: CodegenContext,
18+
renderLiteral: RenderLiteral,
19+
rootType: BoundType | undefined = ctx.resolve(ctx.expr)?.type,
20+
): Map<string, string> {
21+
const out = new Map<string, string>();
22+
if (rootType?.kind !== "struct") return out;
23+
for (const [name, fi] of collectFieldInfo(ctx, rootType)) {
24+
if (fi.defaultValue === undefined) continue;
25+
if (rootType.fields[name]?.kind === "optional") continue;
26+
out.set(name, renderLiteral(fi.defaultValue));
27+
}
28+
return out;
29+
}
30+
31+
/** The rendered default for a binding iff it is a root-level defaulted field. */
32+
export function rootFieldDefault(
33+
binding: Binding | undefined,
34+
defaults: ReadonlyMap<string, string>,
35+
): string | undefined {
36+
if (!binding) return undefined;
37+
const a = binding.access;
38+
if (a.length === 1 && a[0]?.kind === "field") return defaults.get(binding.name);
39+
return undefined;
40+
}

‎packages/core/src/backend/index.ts‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,7 @@ export { generateNipype, NipypeBackend, nipypeNames } from "./nipype/index.js";
2222
export type { PydraNames } from "./pydra/index.js";
2323
export { generatePydra, PydraBackend, pydraNames } from "./pydra/index.js";
2424
export { resolveFieldBinding } from "./resolve-field-binding.js";
25-
export type { OutputEmitPlan } from "./resolve-output-tokens.js";
26-
export {
27-
compactTokens,
28-
isGated,
29-
isIterated,
30-
outputGate,
31-
planOutput,
32-
planScope,
33-
} from "./resolve-output-tokens.js";
25+
export { outputGate } from "./resolve-output-tokens.js";
3426
export { buildEmitModel, generatePython, PythonBackend, renderPythonCall } from "./python/index.js";
3527
export { Scope } from "./scope.js";
3628
export type { JsonSchema } from "./schema/index.js";

‎packages/core/src/backend/python/arg-builder.ts‎

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { Binding, BindingId, BoundType, BoundVariant } from "../../bindings/index.js";
2-
import { collectFieldInfo } from "../collect-field-info.js";
2+
import { collectDefaults, rootFieldDefault } from "../field-defaults.js";
33
import type { Expr } from "../../ir/index.js";
44
import type { CodegenContext } from "../../manifest/index.js";
55
import { CodeBuilder } from "../code-builder.js";
@@ -97,15 +97,6 @@ interface ArgContext {
9797
* carrying a Boutiques default. Restricted to single-segment (root) field access
9898
* so a nested field can never accidentally pick up a same-named root default.
9999
*/
100-
function rootFieldDefault(
101-
binding: Binding,
102-
defaults: ReadonlyMap<string, string>,
103-
): string | undefined {
104-
const a = binding.access;
105-
if (a.length === 1 && a[0]?.kind === "field") return defaults.get(binding.name);
106-
return undefined;
107-
}
108-
109100
/**
110101
* Render a binding's access for an UNCONDITIONAL value read (terminal, repeat
111102
* loop, alternative dispatch): substitutes the field's default via
@@ -145,22 +136,6 @@ function accessOf(binding: Binding, arg: ArgContext, opts: AccessOpts = {}): str
145136
);
146137
}
147138

148-
/**
149-
* Build the field-name -> rendered-default map for a struct root (else empty).
150-
* Includes only non-optional defaulted fields (optional fields are
151-
* presence-guarded; their default comes from the factory's kwarg signature).
152-
*/
153-
function collectDefaults(ctx: CodegenContext, rootType?: BoundType): Map<string, string> {
154-
const out = new Map<string, string>();
155-
if (rootType?.kind !== "struct") return out;
156-
for (const [name, fi] of collectFieldInfo(ctx, rootType)) {
157-
if (fi.defaultValue === undefined) continue;
158-
if (rootType.fields[name]?.kind === "optional") continue;
159-
out.set(name, renderPyLiteral(fi.defaultValue));
160-
}
161-
return out;
162-
}
163-
164139
// -- Recursive descent --
165140

166141
let loopVarCounter = 0;
@@ -179,7 +154,7 @@ export function buildArgs(rootExpr: Expr, ctx: CodegenContext, rootType?: BoundT
179154
joinDepth: 0,
180155
loopVars: new Map(),
181156
valueSubst: new Map(),
182-
defaults: collectDefaults(ctx, rootType),
157+
defaults: collectDefaults(ctx, renderPyLiteral, rootType),
183158
};
184159
return walk(rootExpr, ctx, initialCtx);
185160
}

‎packages/core/src/backend/python/emit.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,21 @@ export function emitDocstring(cb: CodeBuilder, text?: string): void {
2727
cb.line(`"""`);
2828
}
2929

30-
export function emitImports(cb: CodeBuilder, emitOutputs: boolean): void {
30+
export function emitImports(cb: CodeBuilder): void {
3131
cb.line("import dataclasses");
3232
cb.line("import pathlib");
3333
cb.line("import typing");
3434
cb.blank();
35-
const fromStyxdefs = ["Execution", "InputPathType", "Metadata", "Runner", "StyxValidationError"];
36-
if (emitOutputs) fromStyxdefs.push("OutputPathType");
35+
// Every tool emits an Outputs object, so OutputPathType is always needed.
36+
// (Kept last to preserve the previously emitted import order.)
37+
const fromStyxdefs = [
38+
"Execution",
39+
"InputPathType",
40+
"Metadata",
41+
"Runner",
42+
"StyxValidationError",
43+
"OutputPathType",
44+
];
3745
cb.line(`from styxdefs import ${fromStyxdefs.join(", ")}, get_global_runner`);
3846
}
3947

‎packages/core/src/backend/python/outputs-emit.ts‎

Lines changed: 3 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,6 @@
1-
import type {
2-
Binding,
3-
BindingId,
4-
BoundType,
5-
GateAtom,
6-
ResolvedToken,
7-
} from "../../bindings/index.js";
1+
import type { BindingId, BoundType, GateAtom, ResolvedToken } from "../../bindings/index.js";
82
import { outputGate } from "../../bindings/index.js";
9-
import { collectFieldInfo } from "../collect-field-info.js";
3+
import { collectDefaults, rootFieldDefault } from "../field-defaults.js";
104
import type { CodegenContext } from "../../manifest/index.js";
115
import { CodeBuilder } from "../code-builder.js";
126
import {
@@ -89,32 +83,6 @@ interface OutputEmitCtx {
8983
defaults: ReadonlyMap<string, string>;
9084
}
9185

92-
/** The rendered default for a binding iff it is a root-level defaulted field. */
93-
function rootFieldDefault(
94-
binding: Binding | undefined,
95-
defaults: ReadonlyMap<string, string>,
96-
): string | undefined {
97-
if (!binding) return undefined;
98-
const a = binding.access;
99-
if (a.length === 1 && a[0]?.kind === "field") return defaults.get(binding.name);
100-
return undefined;
101-
}
102-
103-
/** Build the field-name -> rendered-default map for the struct root (else empty).
104-
* Includes only non-optional defaulted fields (optional fields are
105-
* presence-guarded; their default comes from the factory's kwarg signature). */
106-
function collectDefaults(ctx: CodegenContext): Map<string, string> {
107-
const out = new Map<string, string>();
108-
const rootType = ctx.resolve(ctx.expr)?.type;
109-
if (rootType?.kind !== "struct") return out;
110-
for (const [name, fi] of collectFieldInfo(ctx, rootType)) {
111-
if (fi.defaultValue === undefined) continue;
112-
if (rootType.fields[name]?.kind === "optional") continue;
113-
out.set(name, renderPyLiteral(fi.defaultValue));
114-
}
115-
return out;
116-
}
117-
11886
interface WrapperRender {
11987
open: string;
12088
loopVar?: string;
@@ -339,7 +307,7 @@ export function emitBuildOutputs(
339307
ctx,
340308
iter: new Map(),
341309
subst: new Map(),
342-
defaults: collectDefaults(ctx),
310+
defaults: collectDefaults(ctx, renderPyLiteral),
343311
};
344312

345313
const fields = collectOutputFields(ctx, pyId);

‎packages/core/src/backend/python/python.ts‎

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -416,22 +416,18 @@ function generatePythonModule(
416416
// Every tool emits an Outputs object: at minimum the synthetic `root` output
417417
// directory (output_file(".")), plus any declared file/mutable outputs and
418418
// stdout/stderr stream fields. OutputPathType is therefore always imported.
419-
const emitOutputs = true;
420-
421-
emitImports(cb, true);
419+
emitImports(cb);
422420
cb.blank();
423421

424422
emitMetadata(ctx, names.metadata, cb);
425423
cb.blank();
426424

427425
emitTypeDeclarations(typeDecls, namedTypes, ctx, cb, names.params, rootTypeTag);
428426

429-
if (emitOutputs) {
430-
emitOutputsClass(ctx, names.outputs, cb);
431-
cb.blank();
432-
}
427+
emitOutputsClass(ctx, names.outputs, cb);
428+
cb.blank();
433429

434-
if (emitOutputs && needsStripExtensionsHelper(ctx)) {
430+
if (needsStripExtensionsHelper(ctx)) {
435431
emitStripExtensionsHelper(cb);
436432
cb.blank();
437433
}
@@ -470,10 +466,8 @@ function generatePythonModule(
470466
emitBuildCargs(ctx, rootType, paramsType, names.cargs, cb);
471467
cb.blank();
472468

473-
if (emitOutputs) {
474-
emitBuildOutputs(ctx, paramsType, names.outputs, names.outputsFn, cb);
475-
cb.blank();
476-
}
469+
emitBuildOutputs(ctx, paramsType, names.outputs, names.outputsFn, cb);
470+
cb.blank();
477471

478472
// Dict-style execute function. For struct roots it's the internal
479473
// `<tool>_execute`; for other roots it doubles as the user-facing wrapper.
@@ -484,8 +478,8 @@ function generatePythonModule(
484478
executeName,
485479
names.metadata,
486480
names.cargs,
487-
emitOutputs ? names.outputsFn : undefined,
488-
emitOutputs ? names.outputs : undefined,
481+
names.outputsFn,
482+
names.outputs,
489483
names.validate,
490484
streamFieldIds(ctx),
491485
cb,
@@ -500,7 +494,7 @@ function generatePythonModule(
500494
names.wrapper,
501495
names.paramsFn,
502496
names.execute,
503-
emitOutputs ? names.outputs : undefined,
497+
names.outputs,
504498
cb,
505499
);
506500
cb.blank();
@@ -510,10 +504,10 @@ function generatePythonModule(
510504
// module's stdlib/styxdefs imports.
511505
const publicSymbols = [
512506
names.params,
513-
...(emitOutputs ? [names.outputs] : []),
507+
names.outputs,
514508
names.metadata,
515509
names.cargs,
516-
...(emitOutputs ? [names.outputsFn] : []),
510+
names.outputsFn,
517511
...(rootIsStruct ? [names.paramsFn, names.execute] : []),
518512
...nestedFactories.map((nf) => nf.funcName),
519513
names.validate,

0 commit comments

Comments
 (0)