Python: fix(declarative): honor the variable field in SetValue

[Somay kousis (Somay-kousis)]

Motivation & Context

The builder accepts variable in place of path for SetValue (ACTION_ALTERNATE_FIELDS["SetValue.path"], _declarative_builder.py:99), and there is a test that checks this builds. But SetValueExecutor only read path, so a SetValue written with variable: built fine and then wrote nothing. No error, the variable just stayed unset.

Description & Review Guide

• What are the major changes? SetValueExecutor now falls back to _get_variable_path when path is missing. That's the same helper SetVariable uses, so both variable: Local.x and variable: {path: Local.x} work. One line in _executors_basic.py, plus tests in test_graph_coverage.py for both shapes and for path winning when both are set.
• What is the impact of these changes? YAML that already uses path: behaves the same. path still wins if both are set. YAML that used variable: now sets the value instead of silently skipping it.
• What do you want reviewers to focus on? Whether honoring the alias is the direction you want. The other option is removing "SetValue.path": ["variable"] from the builder so it fails at build time. I went with the alias because the builder and its test already treat it as valid.

Test before the fix:

>       assert state.get("Local.target") == "test value"
E       AssertionError: assert None == 'test value'
E        +  where None = get('Local.target')
E        +    where get = <agent_framework_declarative._workflows._declarative_base.DeclarativeWorkflowState object at 0x10a76ff20>.get

tests/test_graph_coverage.py:466: AssertionError
=========================== short test summary info ============================
FAILED tests/test_graph_coverage.py::TestBasicExecutorsCoverage::test_set_value_executor_with_variable_field[Local.target]
FAILED tests/test_graph_coverage.py::TestBasicExecutorsCoverage::test_set_value_executor_with_variable_field[variable1]

After:

tests/test_graph_coverage.py::TestBasicExecutorsCoverage::test_set_value_executor_with_variable_field[Local.target] PASSED [ 33%]
tests/test_graph_coverage.py::TestBasicExecutorsCoverage::test_set_value_executor_with_variable_field[variable1] PASSED [ 66%]
tests/test_graph_coverage.py::TestBasicExecutorsCoverage::test_set_value_executor_path_wins_over_variable PASSED [100%]
====================== 3 passed, 181 deselected in 2.13s =======================

uv run poe test -P declarative, with the fix reverted and then with it:

FAILED tests/test_graph_coverage.py::TestBasicExecutorsCoverage::test_set_value_executor_with_variable_field[Local.target]
FAILED tests/test_graph_coverage.py::TestBasicExecutorsCoverage::test_set_value_executor_with_variable_field[variable1]
2 failed, 1152 passed, 501 skipped in 35.17s

1154 passed, 501 skipped in 32.76s

The 501 skips are the PowerFx tests, my machine has no .NET runtime. poe syntax -P declarative and poe typing -P declarative pass.

Related Issue

Fixes #9427

AI Assistance

• No material AI assistance was used.
• This is an AI-assisted contribution. I reviewed, understood, and verified all submitted content and accept responsibility for it.

AI assistance details: An AI assistant helped find the bug, write the fix and test, and draft this description. I ran the repro and the tests myself.

Contribution Checklist

• The code builds clean without any errors or warnings
• All unit tests pass, and I have added new tests where possible
• The PR follows the Contribution Guidelines
• This PR links to an agreed issue with no competing open PR, or the Related Issue section documents a trivial-change or repository-automation exception.
• This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

[Copilot]

🟢 Approval recommended

The narrow fix matches the builder’s existing contract and includes focused regression coverage.

0 open findings

What changed in this PR

Fixes #9427 by making Python’s declarative SetValue executor honor the variable alias already accepted by the builder.

Changes:

• Reuses _get_variable_path when path is absent.
• Adds regression tests for string and nested-object aliases, plus path precedence.

File	Description
python/​packages/​declarative/​tests/​test_graph_coverage.py	Tests both alias forms and explicit-path precedence.
python/​packages/​declarative/​agent_framework_declarative/​_workflows/​_executors_basic.py	Adds the variable-path fallback.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[Bicheng (Kenneth) (BichengWang)]

Reviewed 8cdf12d62ca3b8342a469b29af027906fe66acee against cae49553ce7f1c54edec97ad97fb820fd792175d.

The fallback matches the builder's existing SetValue.path alias contract. Keeping the direct path lookup first preserves precedence when both fields are present, while _get_variable_path handles both the string and nested mapping forms. Value evaluation and completion signaling stay on the existing execution path. I found no correctness regression in the diff.

I checked the regression independently on macOS with Python 3.13.16:

• With the base version of _executors_basic.py, the two new alias cases fail with None instead of the assigned value; the precedence case passes. Restoring this head gives 3 passed.
• The declarative non-integration suite gives 1,154 passed, 501 skipped. The skips are PowerFx-dependent tests; the required runtime is unavailable here. This used the checked-out core and declarative source via PYTHONPATH, pytest 9.1.1, and mcp 2.3.0, without credentials or live service calls.
• I also ran an actual YAML workflow through WorkflowFactory and inspected its final checkpoint. String and nested aliases persist their values, direct path wins without creating the alias target, and false/zero/null values are retained. The same workflow loses the alias assignments with the base executor.

One suggested test improvement: retain a factory/checkpoint regression in the suite for both alias shapes. The new tests exercise the executor with a mocked context, while the existing builder test only checks that the definition builds; a real workflow test would protect their agreement without needing PowerFx or a service.

Before merge, #9427 still needs explicit maintainer agreement under CONTRIBUTING.md: at review time its only reply is automated triage and it has no ready-for-implementation label.

[Somay kousis (Somay-kousis)]

Bicheng (Kenneth) (@BichengWang) thanks for running it end to end. Added that test in 8d8053c: TestWorkflowFactorySetValueAlias in test_workflow_factory.py runs a real YAML workflow through WorkflowFactory with InMemoryCheckpointStorage, for both the string and nested alias, and checks the value lands in a checkpoint.

It's its own class without the PowerFx marker, since the values are literals, so it runs without a .NET runtime. With the base executor both cases fail with assert 'from variable' in [None, None, None, None]; with this branch they pass. Declarative suite: 1,156 passed, 501 skipped.