Severity: HIGH — common path overhead.
Where:
lib/dag/workflow/step.rb:13-73
What:
Constructor walks every nested Hash/Array recursively, dups leaves, freezes everything, allocates a seen cycle-detection hash — unconditionally per Step.new. Configs that come from Loader are produced by Psych (already fresh, no shared refs with caller code); programmatic builders typically pass literals. The deep walk is paranoia in 99% of cases.
Impact / Repro:
Loader builds N Steps per workflow load; tests build many; replace_step builds new Steps. The seen hash is allocated even for trivial scalar configs.
Suggested fix:
Short-circuit when input is already safe: return config if config.frozen? && config.values.all? { |v| v.frozen? || immutable_leaf?(v) }. Or expose Step.unsafe(name:, type:, config:) that skips the walk for trusted callers (Loader uses it).
Filed from a multi-agent review (DRY / efficiency / bug). Behavior change is out of scope; suggested fixes are refactors only.
Severity: HIGH — common path overhead.
Where:
lib/dag/workflow/step.rb:13-73What:
Constructor walks every nested Hash/Array recursively,
dups leaves, freezes everything, allocates aseencycle-detection hash — unconditionally perStep.new. Configs that come from Loader are produced by Psych (already fresh, no shared refs with caller code); programmatic builders typically pass literals. The deep walk is paranoia in 99% of cases.Impact / Repro:
Loader builds N Steps per workflow load; tests build many;
replace_stepbuilds new Steps. Theseenhash is allocated even for trivial scalar configs.Suggested fix:
Short-circuit when input is already safe:
return config if config.frozen? && config.values.all? { |v| v.frozen? || immutable_leaf?(v) }. Or exposeStep.unsafe(name:, type:, config:)that skips the walk for trusted callers (Loader uses it).Filed from a multi-agent review (DRY / efficiency / bug). Behavior change is out of scope; suggested fixes are refactors only.