[legend91019]

Summary

Fixes #979.

Application.step() processes inputs before calling pre_run_step, but Application.astep() previously passed the raw input dictionary to the hook before processing and validation. As a result, the hook could receive undeclared inputs or run before a missing required input was rejected.

This change processes inputs before the async pre-step hook and passes the resulting action_inputs to it, for both synchronous and asynchronous actions executed through astep().

Changes

• burr/core/application.py: call _process_inputs(inputs, next_action) before pre_run_step in _astep() and reuse the processed inputs for async execution.
• tests/core/test_application.py: cover filtering undeclared inputs and rejecting missing required inputs before the hook runs, for sync and async actions.

Verification

Implementation verification previously recorded for this branch:

• tests/core/test_application.py: 144 tests passed.
• tests/core: 432 tests passed.

This description-only update repairs Markdown escaping and corrupted identifiers. It does not change the branch or claim a new test run.

[xihaian251]

Thanks — processing and validating the inputs before pre_run_step is the right direction, and the async path now behaves correctly. The new tests cover both symptoms described in #979.

There is one remaining problem in the synchronous-action path. _astep now processes the inputs before the hooks, but the sync branch still delegates with

output = self._step(inputs=inputs, _run_hooks=False)

and _step calls _process_inputs again on those same inputs. When the action declares a generated dependency (__context or __tracer), the factory therefore runs twice, and the object the hook sees is not the object the action receives.

Reproduction on 3ff45707, with an action that declares __context and a factory that returns a fresh object() on every call. Identical results on Python 3.12.14 and 3.13.14:

astep(), sync action  : 2 factory calls
    AssertionError: __context factory ran 2 times, expected once
    assert <object object at 0x...2860> is <object object at 0x...39B0>   # hook vs action

astep(), async action : 1 factory call, hook and action share the object
step(),  sync action  : 1 factory call, hook and action share the object

So only the sync branch through astep() is affected; the async branch and step() already behave correctly.

#979 lists this as expected behaviour:

The same processed input dictionary should be used by both the hook and the action.

I read this as the last piece of the issue rather than a new concern.

Same root cause, one more data point: astream_result halting on a non-streaming action now processes the inputs three times (it was two on main), because _astep processes them even when _run_hooks=False and _step then processes them again. That is a separate call site and fixing it is not required for #979, but the count does go up with this change.

PR #980 handles the duplicate by threading the already-processed dict into _step through a _processed_inputs parameter. Note that simply passing action_inputs into inputs= does not work: _process_inputs rejects any key starting with __, so feeding it its own output raises ValueError: Inputs starting with a double underscore ... are reserved. Reusing the processed inputs on this branch would be the equivalent fix.

Happy to share the focused regression test — it is one parametrised test covering step(), astep() with a sync action and astep() with an async action, asserting a single factory call plus object identity between the hook and the action. I can open it against your branch or paste it here, whichever you prefer.

---

_{Disclosure: this verification was produced with AI assistance against the pinned commit above. Every number comes from a reproducible local pytest run; happy to provide the test file and exact commands.}

[legend91019]

Thanks @xihaian251 — the sync branch is fixed in 7a58bba, now at the head of this PR.

Fix. _step takes an optional _processed_inputs dict, and the sync branch of _astep passes the dict it already processed:

output = self._step(
    inputs=inputs,
    _run_hooks=False,
    _processed_inputs=action_inputs,
)

Same shape as #980. Your note about feeding action_inputs back through inputs= was the deciding constraint: _process_inputs rejects the __context key, so it had to be a separate parameter rather than a reuse of inputs.

Regression test. Added test_step_and_astep_share_one_processed_inputs_dict in tests/core/test_application.py, parametrised over step(), astep() with a sync action and astep() with an async action. It swaps the __context factory for one that returns a fresh object() per call, then asserts a single factory call plus identity between the object in the hook's inputs and the object the action received.

Negative control, test unchanged, only the production change reverted (Python 3.14.6, Windows):

astep(), sync action : FAILED - __context factory ran 2 times, expected once
step(),  sync action : passed
astep(), async action: passed

That is the same two-case split you reported.

astream_result. The count goes back to main's two instead of the three on 3ff45707, as a side effect of the same change. Counts below instrument _process_inputs itself, so they are direct calls rather than factory invocations:

                       3ff45707   this commit   main
step(),  sync action       1           1          1
astep(), sync action       2           1          1
astep(), async action      1           1          1
astream_result()           3           2          2

I left astream_result itself alone: it still calls _astep(inputs=inputs, _run_hooks=False) with the original dict, so that path processes twice, but no hooks run there, so the identities cannot diverge. Collapsing it to a single call is a separate change if you want it.

Runs. tests/core/test_application.py: 147 passed. tests/core without the Graphviz rendering tests: 427 passed. The four test_graphviz_display.py cases fail in this environment with ExecutableNotFound: failed to execute 'dot' (dot is not on PATH), unrelated to this change. flake8 and isort are clean; black --check flags tests/core/test_application.py, but it flags the base file the same way, and I kept the existing quoting style of the file.

Happy to take your focused test as well if it covers a case mine misses — otherwise I think this one pins the behaviour you reproduced.

[xihaian251]

Thanks for the quick follow-up and for incorporating the feedback.
The _processed_inputs handoff addresses the double-processing mechanism I reported, and the new parametrised test covers the key factory-call and object-identity guarantees.
I agree that the remaining astream_result duplicate-processing issue is separate and should not expand the scope of #979.
I appreciate the careful fix and the negative-control verification.

[legend91019]

Hi @xihaian251 — thanks again for the careful review and for confirming that the _processed_inputs handoff addresses the double-processing mechanism, and for agreeing to keep astream_result out of scope for #979.

Since that was the last open item, would you be able to approve so this can land? Happy to address anything else you'd like to see before merge.

@skrawcz — this one is ready for a maintainer merge whenever convenient: CI green, MERGEABLE, reviewer has signed off on the mechanism and the parametrised regression test.

[xihaian251]

Reviewed the updated head 7a58bba.

The _processed_inputs handoff correctly prevents duplicate input processing in the synchronous-action path of astep(). The parametrized regression test covers step(), astep() with a synchronous action, and astep() with an asynchronous action, verifying both single factory invocation and object identity between the hook and action.

I have no remaining code-level concerns and approve this change.

One CI note: please ensure the full required test workflows have completed before merging. The latest checks visible to me still show Build Burr and Release Validation as action_required, while the documentation workflow reports startup_failure.

[xihaian251]

Cross-checking #995 against #1019, which also touches _astep. Sharing measurements rather than adding another opinion on the code — I replayed both onto current main (3f16c1e6) in separate local worktrees.

Overlap with #980
The production change here is byte-identical to #980's apart from one docstring paragraph, so #980 and #995 are the same fix rather than two designs. #995 is the stronger of the two: it pins the dependency-factory call count and object identity across step, astep + sync action, and astep + async action, which #980 does not assert.

Overlap with #1019
#995 is a strict subset of #1019 in burr/core/application.py. Diffing the two results leaves only the _astep _processed_inputs parameter and the four stream_result/astream_result hunks on #1019's side.

Scope
#995 fixes #979 for astep and leaves the streaming entry points untouched, so #1017 and #1007 stay open after it. Measured on a 19-case cross-entry consistency matrix: main 16 failed / 3 passed; #995 alone 12 failed / 7 passed; #1019 19 passed. So this is a genuine but partial fix — not a criticism of it, just worth knowing before merge.

Suggested sequencing, if maintainers agree
Land #995 for #979 first, then have #1019 rebase and drop its now-duplicated _step/_astep hunks so it becomes purely the streaming-entry fix. I rehearsed that combination locally: tests/core/test_application.py goes to 152 passing, and the cross-entry matrix to 19/19.

One gap either way
No PR currently tests that astream_result's non-streaming halt invokes a dependency factory exactly once. #1019's test_stream_result_shares_one_processed_inputs_dict covers only the sync entry point, while the async halt path depends on the new _astep(_processed_inputs=...) parameter. Happy to supply that test if useful.

CI
Hosted CI has not run on this PR: Build Burr is action_required (awaiting maintainer approval to run) and documentation shows startup_failure on every PR head including unrelated ones, so it carries no signal about this change. All numbers above are local, CPython 3.12, pytest + pytest-asyncio with tests/pytest.ini.