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..b32c1e56df4 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, Sequence 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,155 @@ _ENV_REFERENCE_RE = re.compile(r"\bEnv\.([A-Za-z_][A-Za-z0-9_]*)") +_POWERFX_IDENTIFIER_RE = re.compile(r"[^\W\d]\w*") +_POWERFX_ERROR_RE = re.compile(r"Error (\d+) - (\d+) : (.*?)(?=; Error \d+ - \d+ :|$)", re.DOTALL) + + +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_paths( + formula: str, unrecognized_spans: Sequence[tuple[int, int]] +) -> tuple[set[tuple[str, ...]], set[tuple[str, ...]]]: + """Return input field paths containing an unrecognized-name diagnostic. + + ``inputs.user.name`` yields ``("user", "name")``. 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. + PowerFx resolves local bindings before reporting errors, so valid references to + shadowed ``inputs`` or ``Workflow`` records do not introduce global fields. + """ + inputs_paths: set[tuple[str, ...]] = set() + workflow_inputs_paths: set[tuple[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 not any(root[1] <= start < end <= chain_end for start, end in unrecognized_spans): + continue + if chain[0] == "inputs" and len(chain) > 1: + inputs_paths.add(tuple(chain[1:])) + elif chain[:2] == ["Workflow", "Inputs"] and len(chain) > 2: + workflow_inputs_paths.add(tuple(chain[2:])) + return inputs_paths, workflow_inputs_paths + + +def _complete_with_blanks(record: Mapping[str, Any], paths: set[tuple[str, ...]]) -> dict[str, Any]: + """Return a copy of ``record`` in which every path it does not contain ends in blank. + + Missing parents become records, so ``("user", "name")`` on an empty record gives + ``{"user": {"name": None}}`` and ``inputs.user.name`` stays a valid field access. + A missing field that is also used as a value (``IsBlank(inputs.user)``) is left + missing, because a record would not be blank and a blank record cannot be bound. + Values the record already has are never replaced, and paths below a value that is + not a record are left for PowerFx to report. + """ + completed = dict(record) + children: dict[str, set[tuple[str, ...]]] = {} + for name, *rest in paths: + children.setdefault(name, set()).add(tuple(rest)) + for name, nested in children.items(): + fields = nested - {()} + if name not in completed: + if not fields: + completed[name] = None + elif () not in nested: + completed[name] = _complete_with_blanks({}, fields) + elif fields and isinstance(completed[name], Mapping): + completed[name] = _complete_with_blanks(cast(Mapping[str, Any], completed[name]), fields) + return completed + + +def _bind_missing_input_references( + formula: str, symbols: dict[str, Any], error: ValueError, *, 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. Nested references such as + ``inputs.user.name`` bind ``user`` as a record whose ``name`` is blank. + + ``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_paths, workflow_inputs_paths = _referenced_input_paths(formula, _unrecognized_name_spans(formula, error)) + referenced = workflow_inputs_paths | inputs_paths if inputs_alias_is_canonical else workflow_inputs_paths + bound_inputs = _complete_with_blanks(workflow_input_values, referenced) + if bound_inputs == workflow_input_values: + return symbols + 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 | str) -> 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 + + +def _unrecognized_name_spans(formula: str, error: ValueError) -> list[tuple[int, int]]: + """Read the Python wrapper's diagnostic spans, converting UTF-16 offsets to Python indices. + + Missing or unfamiliar diagnostics leave the original undefined-reference fallback + intact instead of guessing which locally scoped names belong to workflow inputs. + """ + offsets = {0: 0} + utf16_offset = 0 + for index, character in enumerate(formula, 1): + utf16_offset += 2 if ord(character) > 0xFFFF else 1 + offsets[utf16_offset] = index + spans: list[tuple[int, int]] = [] + for match in _POWERFX_ERROR_RE.finditer(str(error)): + if not _is_unrecognized_name_error(match[3]): + continue + start, end = offsets.get(int(match[1])), offsets.get(int(match[2])) + if start is not None and end is not None and start < end: + spans.append((start, end)) + return spans @dataclass(frozen=True) @@ -618,28 +769,31 @@ 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 + unrecognized_error = e + # 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, + unrecognized_error, + 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: + # The blank bindings did not make the formula valid, so keep the + # undefined-reference fallback the formula had without them. + logger.debug(f"PowerFx: blank input bindings did not resolve '{formula}'") # 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 +804,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..62bc0793d4d 100644 --- a/python/packages/declarative/tests/test_powerfx_yaml_compatibility.py +++ b/python/packages/declarative/tests/test_powerfx_yaml_compatibility.py @@ -104,6 +104,180 @@ 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("inputs", [{"other": 1}, {"user": {"id": 1}}]) + @pytest.mark.parametrize( + "expression", + [ + '=If(IsBlank(inputs.user.name), "World", inputs.user.name)', + '=If(IsBlank(Workflow.Inputs.user.name), "World", Workflow.Inputs.user.name)', + ], + ) + async def test_if_with_isblank_when_nested_input_is_missing(self, mock_state, inputs, expression): + """Test that a missing nested input is blank whether or not its parent record was supplied.""" + state = DeclarativeWorkflowState(mock_state) + state.initialize(inputs) + + assert state.eval(expression) == "World" + assert state.get_state_data()["Inputs"] == inputs + + @pytest.mark.parametrize( + "expression", + [ + '=If(IsBlank(inputs.user), "anonymous", "known: " & inputs.user.name)', + "=Sum(inputs.items, Value)", + ], + ) + async def test_unbindable_missing_inputs_keep_undefined_fallback(self, mock_state, expression): + """Test that formulas the blank bindings cannot make valid still evaluate to ``None``.""" + state = DeclarativeWorkflowState(mock_state) + state.initialize({"other": 1}) + + assert state.eval(expression) is None + + @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" + + @pytest.mark.parametrize( + ("expression", "expected"), + [ + ( + ( + '=With({inputs: {name: "local"}}, {Chosen: inputs.name, ' + 'Default: If(IsBlank(Workflow.Inputs.missing), "World", Workflow.Inputs.missing), ' + "Received: Workflow.Inputs})" + ), + {"Chosen": "local", "Default": "World", "Received": {"other": 1, "missing": None}}, + ), + ( + ( + '=With({Workflow: {Inputs: {name: "local"}}}, {Chosen: Workflow.Inputs.name, ' + 'Default: If(IsBlank(inputs.missing), "World", inputs.missing), Received: inputs})' + ), + {"Chosen": "local", "Default": "World", "Received": {"other": 1, "missing": None}}, + ), + ( + ( + '=With({inputs: {name: "local"}, prefix: "😀"}, {Chosen: inputs.name, ' + 'Default: If(IsBlank(Workflow.Inputs.missing), "World", Workflow.Inputs.missing), ' + "Received: Workflow.Inputs})" + ), + {"Chosen": "local", "Default": "World", "Received": {"other": 1, "missing": None}}, + ), + ( + ( + '=With({name: "local"} As inputs, {Chosen: inputs.name, ' + 'Default: If(IsBlank(Workflow.Inputs.missing), "World", Workflow.Inputs.missing), ' + "Received: Workflow.Inputs})" + ), + {"Chosen": "local", "Default": "World", "Received": {"other": 1, "missing": None}}, + ), + ( + ( + '=ForAll([{inputs: {name: "local"}}], {Chosen: inputs.name, ' + 'Default: If(IsBlank(Workflow.Inputs.missing), "World", Workflow.Inputs.missing), ' + "Received: Workflow.Inputs})" + ), + [{"Chosen": "local", "Default": "World", "Received": {"other": 1, "missing": None}}], + ), + ( + ( + '=With({inputs: {name: "local"}}, {Chosen: inputs.unknown, ' + 'Default: If(IsBlank(Workflow.Inputs.missing), "World", Workflow.Inputs.missing), ' + "Received: Workflow.Inputs})" + ), + None, + ), + ], + ) + async def test_local_bindings_do_not_add_global_fields_during_retry(self, mock_state, expression, expected): + """Only missing global input references may extend the workflow record on retry.""" + state = DeclarativeWorkflowState(mock_state) + state.initialize({"other": 1}) + + assert state.eval(expression) == expected + assert state.get_state_data()["Inputs"] == {"other": 1} + + @pytest.mark.parametrize( + "error_message", + [ + "Name isn't valid. 'name' isn't recognized.", + "Power Fx failed compilation: Error 9999 - 10000 : Name isn't valid. 'name' isn't recognized.", + ], + ) + async def test_unknown_diagnostic_format_keeps_undefined_fallback(self, mock_state, monkeypatch, error_message): + """Missing or invalid diagnostic locations must not introduce speculative input fields.""" + state = DeclarativeWorkflowState(mock_state) + state.initialize({"other": 1}) + evaluate = MagicMock(side_effect=[ValueError(error_message), "unexpected retry"]) + monkeypatch.setattr(state, "_eval_powerfx", evaluate) + + assert state.eval('=If(IsBlank(inputs.name), "World", inputs.name)') is None + evaluate.assert_called_once() + 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..36d7e942985 100644 --- a/python/packages/declarative/tests/test_workflow_factory.py +++ b/python/packages/declarative/tests/test_workflow_factory.py @@ -486,6 +486,42 @@ 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_missing_nested_input_uses_isblank_default(self): + """Test that ``IsBlank`` defaults apply to nested inputs whose parent record was omitted.""" + factory = WorkflowFactory() + workflow = factory.create_workflow_from_yaml(""" +name: missing-nested-input-test +actions: + - kind: SendActivity + activity: + text: '=If(IsBlank(inputs.user.name), "World", inputs.user.name)' +""") + + result = await workflow.run({"other": 1}) + + assert any("World" 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!")