Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
190 changes: 190 additions & 0 deletions scratchpads/optional-input-resolution/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
# Bug Report: Optional Inputs Without Defaults Fail Template Resolution

## Summary

When a workflow input is declared with `required: false` but no `default` value, and the user doesn't provide a value, any template referencing that input fails with "Unresolved variables".

Expected behavior: Optional inputs should resolve to `null` or empty string when not provided.

## Problem Statement

Users declare optional inputs to allow flexible workflow usage. However, if they reference an optional input in a template and the user doesn't provide a value, the workflow fails - defeating the purpose of making it optional.

### Current Behavior

```json
{
"inputs": {
"optional_param": {"type": "string", "required": false}
},
"nodes": [{
"id": "test",
"type": "shell",
"params": {
"stdin": "${optional_param}",
"command": "cat"
}
}]
}
```

Running without the optional param:
```
$ pflow workflow.json

❌ Workflow execution failed
Error: Unresolved variables in parameter 'stdin': ${optional_param}
```

### Expected Behavior

The workflow should succeed with `${optional_param}` resolving to `null` or empty string:

```
$ pflow workflow.json
✓ Workflow completed
(empty output)
```

## Steps to Reproduce

### Test 1: Optional input without default (FAILS)

```json
{
"inputs": {
"optional_name": {"type": "string", "required": false}
},
"nodes": [{
"id": "greet",
"type": "shell",
"params": {"stdin": "${optional_name}", "command": "cat"}
}],
"edges": []
}
```

```bash
pflow test.json # No value provided
# Result: FAILS with "Unresolved variables"
```

### Test 2: Optional input WITH default (WORKS)

```json
{
"inputs": {
"optional_name": {"type": "string", "required": false, "default": "World"}
},
"nodes": [{
"id": "greet",
"type": "shell",
"params": {"stdin": "${optional_name}", "command": "cat"}
}],
"edges": []
}
```

```bash
pflow test.json # No value provided
# Result: WORKS, outputs "World"
```

### Test 3: Mixed templates - misleading error (FAILS)

```json
{
"inputs": {
"provided": {"type": "string", "required": true},
"missing": {"type": "string", "required": false}
},
"nodes": [{
"id": "test",
"type": "shell",
"params": {
"stdin": {"a": "${provided}", "b": "${missing}"},
"command": "jq ."
}
}],
"edges": []
}
```

```bash
pflow test.json provided="hello"
```

**Actual error:**
```
Error: Unresolved variables in parameter 'stdin': ${provided}, ${missing}

Available context keys:
• provided (str): hello
```

**Issue:** Error says `${provided}` is unresolved, but it's clearly in context. The error should only list `${missing}`.

## Root Cause Analysis

1. Optional inputs without defaults are **not added to the context** at all
2. Template resolution **fails on first unresolvable variable**
3. Error reporting **lists all templates** in the parameter as unresolved, not just the problematic ones

## Impact

- **Workaround tax:** Users must always provide `default` for optional inputs, even when empty string isn't semantically correct
- **Confusing errors:** Error message suggests ALL templates failed, even ones with valid values
- **Design limitation:** Can't distinguish between "user provided empty string" vs "user didn't provide value"

## Proposed Solutions

### Option A: Resolve missing optional inputs to null (Recommended)

When an optional input has no default and no value is provided:
- Add it to context with value `null`
- Templates like `${optional_param}` resolve to `null` (or empty string in string contexts)

**Pros:** Most intuitive behavior, matches how optional parameters work in most languages
**Cons:** Might break workflows that rely on current "fail if missing" behavior

### Option B: Resolve to empty string

Similar to Option A but use empty string `""` instead of `null`.

**Pros:** Simpler, no null handling needed
**Cons:** Can't distinguish between "not provided" and "provided empty"

### Option C: Add explicit null syntax

Allow workflows to handle missing values explicitly:
```json
"stdin": "${optional_param ?? 'default_value'}"
```

**Pros:** Maximum flexibility
**Cons:** More complex, changes template syntax

## Secondary Issue: Misleading Error Message

When multiple templates exist and ONE is unresolvable, the error lists ALL as unresolved:

```
Error: Unresolved variables in parameter 'stdin': ${provided}, ${missing}
```

Should only list actually unresolvable ones:
```
Error: Unresolved variables in parameter 'stdin': ${missing}
```

This is lower priority but would help debugging.

## Test Cases

See `test-cases/` directory for reproduction workflows.

## Environment

- pflow version: (current)
- OS: macOS (Darwin 24.6.0)
- Date discovered: 2026-01-08
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"_test": "Optional input without default - currently FAILS, should PASS",
"_run": "pflow 01-optional-no-default.json",
"_expected": "Should succeed with empty/null output",
"inputs": {
"optional_name": {
"type": "string",
"required": false,
"description": "Optional name with no default"
}
},
"nodes": [
{
"id": "greet",
"type": "shell",
"params": {
"stdin": "${optional_name}",
"command": "echo \"Got: $(cat)\""
}
}
],
"edges": []
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
{
"_test": "Optional input with default - currently WORKS (baseline)",
"_run": "pflow 02-optional-with-default.json",
"_expected": "Should succeed with 'World' output",
"inputs": {
"optional_name": {
"type": "string",
"required": false,
"default": "World",
"description": "Optional name with default"
}
},
"nodes": [
{
"id": "greet",
"type": "shell",
"params": {
"stdin": "${optional_name}",
"command": "echo \"Got: $(cat)\""
}
}
],
"edges": []
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{
"_test": "Mixed resolvable and unresolvable templates - shows misleading error",
"_run": "pflow 03-mixed-templates.json provided=hello",
"_expected": "Should succeed OR error should only mention ${missing}, not ${provided}",
"inputs": {
"provided": {
"type": "string",
"required": true
},
"missing": {
"type": "string",
"required": false
}
},
"nodes": [
{
"id": "test",
"type": "shell",
"params": {
"stdin": {
"has_value": "${provided}",
"no_value": "${missing}"
},
"command": "jq ."
}
}
],
"edges": []
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
{
"_test": "Real-world example: optional output path",
"_run": "pflow 04-real-world-example.json url=https://example.com",
"_expected": "Should work - use auto-generated filename when output_path not provided",
"_notes": "This pattern is common: allow user to override output location, but have sensible default",
"inputs": {
"url": {
"type": "string",
"required": true,
"description": "URL to fetch"
},
"output_path": {
"type": "string",
"required": false,
"description": "Optional custom output path"
}
},
"nodes": [
{
"id": "generate-default-path",
"type": "shell",
"params": {
"command": "echo './output.txt'"
}
},
{
"id": "determine-path",
"type": "shell",
"purpose": "Use custom path if provided, otherwise use default",
"params": {
"stdin": {
"custom": "${output_path}",
"default": "${generate-default-path.stdout}"
},
"command": "jq -r 'if .custom != \"\" and .custom != null then .custom else .default end'"
}
},
{
"id": "save-result",
"type": "shell",
"params": {
"command": "echo 'Would save to: ${determine-path.stdout}'"
}
}
],
"edges": [
{
"from": "generate-default-path",
"to": "determine-path"
},
{
"from": "determine-path",
"to": "save-result"
}
]
}
7 changes: 6 additions & 1 deletion src/pflow/runtime/node_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,11 @@ def _build_enhanced_template_error(self, param_key: str, template: str, context:
Formatted error message with context and suggestions
"""
# Extract variable names from template
variables = TemplateResolver.extract_variables(str(template))
all_variables = TemplateResolver.extract_variables(str(template))

# Filter to only actually unresolved variables (not in context)
# This prevents misleading errors like "${provided}, ${missing}" when only ${missing} failed
variables = {v for v in all_variables if not TemplateResolver.variable_exists(v, context)}

# Build available keys section
available_keys = [k for k in context if not k.startswith("__")]
Expand All @@ -758,6 +762,7 @@ def _build_enhanced_template_error(self, param_key: str, template: str, context:
available_display = available_keys

# Simplified single-line error message (removes redundancy)
# Only report actually unresolved variables
error_parts = [f"Unresolved variables in parameter '{param_key}': {', '.join(f'${{{v}}}' for v in variables)}"]

# Add available keys section (only if there are keys to show)
Expand Down
10 changes: 8 additions & 2 deletions src/pflow/runtime/workflow_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,11 +173,17 @@ def prepare_inputs(
)
defaults[input_name] = default_value
else:
# Optional with no default key means it can be omitted entirely
# Optional inputs without explicit default resolve to None.
# Rationale: "required: false" means "can be omitted", and omitted
# values should still be available in context (as None) so templates
# like ${optional_param} can resolve rather than fail validation.
# Note: Nested access like ${optional_param.field} will still fail
# at runtime since you can't traverse into None - this is intentional.
logger.debug(
f"Optional input '{input_name}' not provided and has no default",
f"Optional input '{input_name}' not provided, using None as default",
extra={"phase": "input_validation", "input": input_name},
)
defaults[input_name] = None
else:
# Input is provided
logger.debug(f"Input '{input_name}' provided", extra={"phase": "input_validation", "input": input_name})
Expand Down
Loading
Loading