diff --git a/python/packages/declarative/agent_framework_declarative/_workflows/_declarative_base.py b/python/packages/declarative/agent_framework_declarative/_workflows/_declarative_base.py index 7338aa62260..360eab513b4 100644 --- a/python/packages/declarative/agent_framework_declarative/_workflows/_declarative_base.py +++ b/python/packages/declarative/agent_framework_declarative/_workflows/_declarative_base.py @@ -31,7 +31,7 @@ import re import sys import uuid -from collections.abc import Iterator, Mapping +from collections.abc import Callable, Iterator, Mapping from dataclasses import dataclass, field from decimal import Decimal as _Decimal from enum import Enum @@ -47,7 +47,9 @@ from ._errors import DeclarativeWorkflowError from ._powerfx_functions import ( + _after_trailing_trivia, # pyright: ignore[reportPrivateUsage] _iter_powerfx_expression_indices, # pyright: ignore[reportPrivateUsage] + _skip_powerfx_opaque_token, # pyright: ignore[reportPrivateUsage] whole_call_args, ) from ._powerfx_limits import _PowerFxStateBudget, _validate_powerfx_state # pyright: ignore[reportPrivateUsage] @@ -70,6 +72,101 @@ _ENV_REFERENCE_RE = re.compile(r"\bEnv\.([A-Za-z_][A-Za-z0-9_]*)") +_POWERFX_IDENTIFIER_RE = re.compile(r"[^\W\d]\w*") + + +def _read_powerfx_identifier(formula: str, start: int) -> tuple[str, int] | None: + """Read a plain or single-quoted PowerFx identifier at ``start``. + + Returns the identifier name and the index after it, or ``None`` when no + identifier starts there. + """ + if formula.startswith("'", start): + end = _skip_powerfx_opaque_token(formula, start) + if end - start < 3 or formula[end - 1] != "'": + return None + return formula[start + 1 : end - 1].replace("''", "'"), end + match = _POWERFX_IDENTIFIER_RE.match(formula, start) + return (match.group(), match.end()) if match else None + + +def _read_member_access(formula: str, start: int) -> tuple[str, int] | None: + """Read ``.`` at ``start``, allowing whitespace and comments around the dot.""" + dot = _after_trailing_trivia(formula, start) + if not formula.startswith(".", dot): + return None + return _read_powerfx_identifier(formula, _after_trailing_trivia(formula, dot + 1)) + + +def _referenced_input_names(formula: str) -> tuple[set[str], set[str]]: + """Return the fields referenced as ``inputs.`` and as ``Workflow.Inputs.`` in code. + + String literals and comments are skipped, so input-like text inside them is not a reference. + Only identifiers that start a member-access chain count, so ``Local.inputs.name`` is not an + ``inputs`` reference. + """ + inputs_names: set[str] = set() + workflow_inputs_names: set[str] = set() + chain_end = 0 + for cursor in _iter_powerfx_expression_indices(formula): + if cursor < chain_end: + continue + if cursor > 0 and (formula[cursor - 1].isalnum() or formula[cursor - 1] == "_"): + continue + root = _read_powerfx_identifier(formula, cursor) + if root is None: + continue + chain = [root[0]] + chain_end = root[1] + while (member := _read_member_access(formula, chain_end)) is not None: + chain.append(member[0]) + chain_end = member[1] + if chain[0] == "inputs" and len(chain) > 1: + inputs_names.add(chain[1]) + elif chain[:2] == ["Workflow", "Inputs"] and len(chain) > 2: + workflow_inputs_names.add(chain[2]) + return inputs_names, workflow_inputs_names + + +def _bind_missing_input_references( + formula: str, symbols: dict[str, Any], *, inputs_alias_is_canonical: bool +) -> dict[str, Any]: + """Expose inputs the formula references but the caller did not supply as blank. + + PowerFx rejects ``inputs.name`` at compile time when ``name`` is not a field of the + inputs record, which turns the whole expression into ``None`` and defeats defaults + such as ``If(IsBlank(inputs.name), "World", inputs.name)``. Binding the missing + fields to blank only for this evaluation makes them behave like an explicit ``None`` + input without changing the workflow state. + + ``inputs`` is an alias of ``Workflow.Inputs`` unless a custom ``inputs`` namespace + replaces it; in that case the two records are completed independently and the custom + namespace is left unchanged. + """ + workflow = symbols.get("Workflow") + if not isinstance(workflow, Mapping): + return symbols + workflow_values = cast(Mapping[str, Any], workflow) + workflow_inputs = workflow_values.get("Inputs") + if not isinstance(workflow_inputs, Mapping): + return symbols + workflow_input_values = cast(Mapping[str, Any], workflow_inputs) + inputs_names, workflow_inputs_names = _referenced_input_names(formula) + referenced = workflow_inputs_names | inputs_names if inputs_alias_is_canonical else workflow_inputs_names + missing = {name for name in referenced if name not in workflow_input_values} + if not missing: + return symbols + bound_inputs: dict[str, Any] = {**workflow_input_values, **dict.fromkeys(sorted(missing))} + bound = {**symbols, "Workflow": {**workflow_values, "Inputs": bound_inputs}} + if inputs_alias_is_canonical: + bound["inputs"] = bound_inputs + return bound + + +def _is_unrecognized_name_error(error: ValueError) -> bool: + """Return whether a PowerFx error reports a name or field that does not resolve.""" + message = str(error) + return "isn't recognized" in message or "Name isn't valid" in message @dataclass(frozen=True) @@ -618,28 +715,28 @@ def eval(self, expression: str) -> Any: except locale.Error: continue - engine = Engine() - try: - from System.Globalization import ( # pyright: ignore[reportMissingImports] - CultureInfo, # pyright: ignore[reportUnknownVariableType] - ) - except ImportError: - return engine.eval(formula, symbols=symbols, locale=_POWERFX_EVAL_LOCALE) - - original_culture = cast(Any, CultureInfo.CurrentCulture) # pyright: ignore[reportUnknownMemberType] try: - CultureInfo.CurrentCulture = CultureInfo(_POWERFX_EVAL_LOCALE) - return engine.eval(formula, symbols=symbols, locale=_POWERFX_EVAL_LOCALE) - finally: - CultureInfo.CurrentCulture = original_culture - except ValueError as e: - error_msg = str(e) + return self._eval_powerfx(Engine, formula, symbols) + except ValueError as e: + if not _is_unrecognized_name_error(e): + raise + # The formula may only fail because it references inputs the caller omitted. + # Retry with those inputs bound to blank so IsBlank defaults apply. + bound_symbols = _bind_missing_input_references( + formula, + symbols, + inputs_alias_is_canonical="inputs" not in self.get_state_data().get("Custom", {}), + ) + if bound_symbols is not symbols: + try: + return self._eval_powerfx(Engine, formula, bound_symbols) + except ValueError as e: + if not _is_unrecognized_name_error(e): + raise # Handle undefined variable errors gracefully by returning None # This matches the behavior of the legacy fallback parser - if "isn't recognized" in error_msg or "Name isn't valid" in error_msg: - logger.debug(f"PowerFx: undefined variable in expression '{formula}', returning None") - return None - raise + logger.debug(f"PowerFx: undefined variable in expression '{formula}', returning None") + return None finally: locale.setlocale(locale.LC_NUMERIC, original_numeric_locale) finally: @@ -650,6 +747,24 @@ def eval(self, expression: str) -> Any: else: self.set(path, previous) + @staticmethod + def _eval_powerfx(engine_type: Callable[[], Any], formula: str, symbols: dict[str, Any]) -> Any: + """Evaluate ``formula`` with a fresh PowerFx engine in the evaluation culture.""" + engine = engine_type() + try: + from System.Globalization import ( # pyright: ignore[reportMissingImports] + CultureInfo, # pyright: ignore[reportUnknownVariableType] + ) + except ImportError: + return engine.eval(formula, symbols=symbols, locale=_POWERFX_EVAL_LOCALE) + + original_culture = cast(Any, CultureInfo.CurrentCulture) # pyright: ignore[reportUnknownMemberType] + try: + CultureInfo.CurrentCulture = CultureInfo(_POWERFX_EVAL_LOCALE) + return engine.eval(formula, symbols=symbols, locale=_POWERFX_EVAL_LOCALE) + finally: + CultureInfo.CurrentCulture = original_culture + def _eval_custom_function(self, formula: str) -> Any | None: """Handle custom functions not supported by the Python PowerFx library. diff --git a/python/packages/declarative/tests/test_powerfx_yaml_compatibility.py b/python/packages/declarative/tests/test_powerfx_yaml_compatibility.py index 308982c6324..01a363e65f3 100644 --- a/python/packages/declarative/tests/test_powerfx_yaml_compatibility.py +++ b/python/packages/declarative/tests/test_powerfx_yaml_compatibility.py @@ -104,6 +104,73 @@ async def test_if_with_isblank(self, mock_state): result = state.eval('=If(IsBlank(Workflow.Inputs.name), "World", Workflow.Inputs.name)') assert result == "Alice" + @pytest.mark.parametrize( + "expression", + [ + '=If(IsBlank(inputs.name), "World", inputs.name)', + '=If(IsBlank(Workflow.Inputs.name), "World", Workflow.Inputs.name)', + "=If(IsBlank(inputs.'name'), \"World\", inputs.'name')", + "=If(IsBlank(Workflow.Inputs.'name'), \"World\", Workflow.Inputs.'name')", + '=If(IsBlank(inputs . name), "World", inputs /* field */ . name)', + '=If(IsBlank(Workflow . Inputs . name), "World", Workflow // root\n.Inputs.name)', + '=$"{If(IsBlank(inputs.name), "World", inputs.name)}"', + ], + ) + async def test_if_with_isblank_when_input_is_missing(self, mock_state, expression): + """Test that an input the caller did not supply is blank, so IsBlank defaults apply.""" + state = DeclarativeWorkflowState(mock_state) + state.initialize({"other": 1}) + + assert state.eval(expression) == "World" + assert state.eval("=IsBlank(inputs.name)") is True + # The missing input is only bound for evaluation, not written to state. + assert "name" not in state.get_state_data()["Inputs"] + + @pytest.mark.parametrize( + "expression", + [ + "=inputs /* inputs.name */", + "=inputs // Workflow.Inputs.name", + '=If(Len("inputs.name") > 0, inputs, inputs)', + '=If(Len($"Workflow.Inputs.name") > 0, inputs, inputs)', + ], + ) + async def test_input_references_in_comments_and_literals_are_not_bound(self, mock_state, expression): + """Test that input-like text in comments and string literals does not add blank inputs.""" + state = DeclarativeWorkflowState(mock_state) + state.initialize({"other": 1}) + + assert state.eval(expression) == {"other": 1} + + @pytest.mark.parametrize( + ("expression", "expected"), + [ + ('=If(Local . inputs . name = "local", inputs, inputs)', {"other": 1}), + ('=If(Local./* member */inputs.name = "local", inputs, inputs)', {"other": 1}), + ( + '=With({inputs: {name: "local"}}, {Chosen: inputs.name, Received: Workflow.Inputs})', + {"Chosen": "local", "Received": {"other": 1}}, + ), + ], + ) + async def test_member_names_and_local_bindings_are_not_workflow_inputs(self, mock_state, expression, expected): + """Test that ``inputs`` as a member name or a local binding does not add blank workflow inputs.""" + state = DeclarativeWorkflowState(mock_state) + state.initialize({"other": 1}) + state.set("Local.inputs", {"name": "local"}) + + assert state.eval(expression) == expected + + async def test_custom_inputs_namespace_keeps_workflow_inputs(self, mock_state): + """Test that a custom ``inputs`` namespace does not replace the workflow inputs record.""" + state = DeclarativeWorkflowState(mock_state) + state.initialize({"name": "Alice"}) + state.set("inputs.other", 1) + + assert state.eval("=Workflow.Inputs.name") == "Alice" + assert state.eval("=inputs.other") == 1 + assert state.eval('=If(IsBlank(Workflow.Inputs.missing), "World", Workflow.Inputs.missing)') == "World" + async def test_not_function(self, mock_state): """Test Not function.""" state = DeclarativeWorkflowState(mock_state) diff --git a/python/packages/declarative/tests/test_workflow_factory.py b/python/packages/declarative/tests/test_workflow_factory.py index 46b3d11a9e1..9e1722d3a80 100644 --- a/python/packages/declarative/tests/test_workflow_factory.py +++ b/python/packages/declarative/tests/test_workflow_factory.py @@ -486,6 +486,26 @@ async def test_execute_if_workflow(self): _text_outputs = [str(o) for o in outputs if isinstance(o, str) or hasattr(o, "data")] # noqa: F841 assert any("Condition was true" in str(o) for o in outputs) + @_requires_powerfx + async def test_custom_inputs_namespace_does_not_hide_workflow_inputs(self): + """Test that writing ``inputs.`` keeps the supplied ``Workflow.Inputs`` values readable.""" + factory = WorkflowFactory() + workflow = factory.create_workflow_from_yaml(""" +name: custom-inputs-namespace-test +actions: + - kind: SetValue + id: set_custom_inputs + path: inputs.other + value: 1 + - kind: SendActivity + activity: + text: =Workflow.Inputs.name +""") + + result = await workflow.run({"name": "Alice"}) + + assert any("Alice" in str(output) for output in result.get_outputs()) + @pytest.mark.asyncio async def test_entry_join_executor_initializes_workflow_inputs(self): """Regression test for #3948: Entry JoinExecutor must initialize Workflow.Inputs. diff --git a/python/samples/03-workflows/declarative/simple_workflow/main.py b/python/samples/03-workflows/declarative/simple_workflow/main.py index 4deb1532f12..5499a4de9b8 100644 --- a/python/samples/03-workflows/declarative/simple_workflow/main.py +++ b/python/samples/03-workflows/declarative/simple_workflow/main.py @@ -25,6 +25,8 @@ async def main() -> None: # Run with a custom name print("\nRunning with custom name 'Alice':") result = await workflow.run({"name": "Alice"}) + for output in result.get_outputs(): + print(f" Output: {output}") print("\n" + "-" * 40) print("Workflow completed!")