Skip to content

Commit 18e7354

Browse files
committed
Address fifteenth review: fan_in docstring, gate defaults, validation guards, reserved prefix
- FanInStep docstring: aggregate-only, no blocking semantics - FanInStep: guard output_config as dict, handle None - Gate validate: use same default options as execute - Validate inputs is dict and steps is list before iterating - Reserve _fanout_ prefix in step ID validation - PUBLISHING.md: remove unenforced checklist items, add _fanout_ note
1 parent c32a4ce commit 18e7354

File tree

4 files changed

+31
-17
lines changed

4 files changed

+31
-17
lines changed

src/specify_cli/workflows/engine.py

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -115,18 +115,24 @@ def validate_workflow(definition: WorkflowDefinition) -> list[str]:
115115
errors.append("Workflow is missing 'workflow.version'.")
116116

117117
# -- Inputs -----------------------------------------------------------
118-
for input_name, input_def in definition.inputs.items():
119-
if not isinstance(input_def, dict):
120-
errors.append(f"Input {input_name!r} must be a mapping.")
121-
continue
122-
input_type = input_def.get("type")
123-
if input_type and input_type not in ("string", "number", "boolean"):
124-
errors.append(
125-
f"Input {input_name!r} has invalid type {input_type!r}. "
126-
f"Must be 'string', 'number', or 'boolean'."
127-
)
118+
if not isinstance(definition.inputs, dict):
119+
errors.append("'inputs' must be a mapping (or omitted).")
120+
else:
121+
for input_name, input_def in definition.inputs.items():
122+
if not isinstance(input_def, dict):
123+
errors.append(f"Input {input_name!r} must be a mapping.")
124+
continue
125+
input_type = input_def.get("type")
126+
if input_type and input_type not in ("string", "number", "boolean"):
127+
errors.append(
128+
f"Input {input_name!r} has invalid type {input_type!r}. "
129+
f"Must be 'string', 'number', or 'boolean'."
130+
)
128131

129132
# -- Steps ------------------------------------------------------------
133+
if not isinstance(definition.steps, list):
134+
errors.append("'steps' must be a list.")
135+
return errors
130136
if not definition.steps:
131137
errors.append("Workflow has no steps defined.")
132138

@@ -154,6 +160,11 @@ def _validate_steps(
154160
errors.append("Step is missing 'id' field.")
155161
continue
156162

163+
if step_id.startswith("_fanout_"):
164+
errors.append(
165+
f"Step ID {step_id!r} uses reserved prefix '_fanout_'."
166+
)
167+
157168
if step_id in seen_ids:
158169
errors.append(f"Duplicate step ID {step_id!r}.")
159170
seen_ids.add(step_id)

src/specify_cli/workflows/steps/fan_in/__init__.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,20 @@
99

1010

1111
class FanInStep(StepBase):
12-
"""Join point — blocks until all ``wait_for:`` steps complete.
12+
"""Join point that aggregates results from ``wait_for:`` steps.
1313
14-
Aggregates their results into ``fan_in.results``.
14+
Reads completed step outputs from ``context.steps`` and collects
15+
them into ``output.results``. Does not block; relies on the
16+
engine executing steps sequentially.
1517
"""
1618

1719
type_key = "fan-in"
1820

1921
def execute(self, config: dict[str, Any], context: StepContext) -> StepResult:
2022
wait_for = config.get("wait_for", [])
21-
output_config = config.get("output", {})
23+
output_config = config.get("output") or {}
24+
if not isinstance(output_config, dict):
25+
output_config = {}
2226

2327
# Collect results from referenced steps
2428
results = []

src/specify_cli/workflows/steps/gate/__init__.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,8 @@ def validate(self, config: dict[str, Any]) -> list[str]:
9292
errors.append(
9393
f"Gate step {config.get('id', '?')!r} is missing 'message' field."
9494
)
95-
options = config.get("options", [])
96-
if options and not isinstance(options, list):
95+
options = config.get("options", ["approve", "reject"])
96+
if not isinstance(options, list):
9797
errors.append(
9898
f"Gate step {config.get('id', '?')!r}: 'options' must be a list."
9999
)

workflows/PUBLISHING.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,7 @@ steps:
9393
- ✅ Step types are valid: `command`, `prompt`, `shell`, `gate`, `if`, `switch`, `while`, `do-while`, `fan-out`, `fan-in`
9494
- ✅ Required fields present per step type (e.g., `condition` for `if`, `expression` for `switch`)
9595
- ✅ Input types are valid: `string`, `number`, `boolean`
96-
- ✅ Enum values match declared type
97-
- ✅ Expression references (`{{ steps.*.output.* }}`) point to steps that run before they're referenced
96+
- ✅ Step IDs do not start with `_fanout_` (reserved prefix)
9897

9998
### 3. Test Locally
10099

0 commit comments

Comments
 (0)