Repository navigation
[fix] keep falsy tool results (0, False, []) on the sync tool execution path - #9948
Conversation
…on path
run_function_call stringified the tool result only when it was truthy, so a
tool returning 0, 0.0, False, [] or {} reached the model as an empty tool
message. arun_function_calls already used str(result), so agent.run() and
agent.arun() gave the model different results for the same tool.
Fixes agno-agi#9947
VANDRANKI
left a comment
VANDRANKI
left a comment
There was a problem hiding this comment.
Community review, not a merge gate.
Traced this fully. The bug: libs/agno/agno/models/base.py's sync run_function_call had function_call_output = str(function_execution_result.result) if function_execution_result.result else "", which treats any falsy tool result (0, False, [], {}, 0.0) as "no result" and silently replaces it with an empty string before it reaches the model.
I checked the async path (arun_function_calls, around line 3037-3050 in the current main branch) and it already does function_call_output = str(function_call.result) with no truthiness check at all. So this PR is making the sync path match an async behavior that already existed, not introducing new behavior. I also checked FunctionExecutionResult in libs/agno/agno/tools/function.py: result defaults to None and is set to whatever the user's tool entrypoint returns, including None for a void tool. Both before and after this PR, a None result stringifies differently on each path ("" on sync pre-fix, "None" on async both before and after) - this PR doesn't change that, it only aligns the falsy-but-non-None cases (0, False, [], {}) which is exactly what the new test file and its test_sync_and_async_tools_match parametrized case checks.
The new test file (test_tool_result_falsy_values.py) covers both sync and async paths with 0, 0.0, False, [], {}, 1, "text" and asserts the stringified content matches on both paths. I traced the fix, the async comparison, and the test cases by hand and didn't find a gap.
harshsinha03
left a comment
harshsinha03
left a comment
There was a problem hiding this comment.
LGTM! Thanks for contributing
## Summary Workflow step content checks used truthiness, so valid step results such as a zero count or False were omitted when building downstream context. Treat content as present unless it is None or blank text (`content is not None and str(content).strip()`, the check `Parallel` already used for its combined output) in: - `StepInput.get_all_previous_content` - `StepInput.get_step_content` for Parallel sub-steps and steps nested inside them - `Parallel._build_aggregated_content` - `Step._get_deepest_content_from_step_output`, whose return type now matches the non-string content it returns Add regressions for zero, False, empty containers, blank text, absent content, ordering and the Parallel paths. No existing issue is linked; this behavior was reproduced from the current main branch. ## Type of change - [x] Bug fix - [ ] New feature - [ ] Breaking change - [ ] Improvement - [ ] Model update - [ ] Other ## Validation New tests: 7 failed and 4 passed against the unchanged base; all 11 passed with the patch. Including existing StepInput serialization tests: 15 passed. No external services or model calls. ```text python -m pytest libs/agno/tests/unit/workflow/test_previous_content_values.py libs/agno/tests/unit/workflow/test_step_input_serialization.py -q ``` Changed-file Ruff format/check and source-file mypy pass. On Windows, the underlying checks from scripts/format.sh and scripts/validate.sh were run directly: repository-wide Ruff lint/import checks and the cookbook pattern check pass; mypy passes for all 1055 agno and 21 agnoctl source files. Repository-wide format checking reports one unchanged baseline file, libs/agno/tests/unit/vectordb/test_elasticsearch.py (also reproduced on pristine main); changed files pass. Bash scripts themselves and the complete test suite were not run. Python 3.12 on Windows; pytest 9.1.1, pytest-asyncio 1.4.0, Ruff 0.15.20, mypy 2.1.0. ## Checklist - [x] Code complies with style guidelines (changed files checked) - [ ] Ran full format/validation scripts (`./scripts/format.sh` and `./scripts/validate.sh`) - [ ] Contributor self-review completed - [ ] Documentation updated (not part of this patch) - [ ] Examples and guides updated (not part of this patch) - [ ] Tested in a clean, dependency-consistent environment - [x] Tests added/updated ## Duplicate and AI-Generated PR Check - [x] Searched existing open PR titles and relevant issue/PR keywords on 2026-09-16; no matching fix was found. Recheck immediately before submission. - [ ] If a similar PR exists, explain why this PR is a better approach (related but different fixes discussed below) - [x] This patch and PR draft were entirely AI-generated ## Additional Notes Empty strings now retain the step heading; None continues to be omitted. This semantic choice needs maintainer review. Related PR #9948 fixes model tool-result formatting in models/base.py, not this workflow helper. Other workflow content accessors are outside this patch. An AI coding assistant discovered the behavior, drafted the patch/tests and executed the reported local checks. The contributor approved submission of this patch. The reported implementation and validation work was performed by the assistant; no independent human implementation claim is made. --------- Co-authored-by: Harsh <[email protected]> Co-authored-by: Harsh Sinha <[email protected]>
- NOTES 补 bytedance/deer-flow#5509、agno-agi/agno#9948 两条中文说明（此前 cron 已把标题行写进贡献区但没有说明） - 缺陷表：deer-flow #5473 → 社区 PR #5494 已修复；openai-agents-python #5041 → 维护者未接受并关闭（需真实 provider wire 证据） - deer-flow star 数 82k → 83k
Summary
On the synchronous tool-execution path, a tool whose return value is falsy but meaningful (
0,0.0,False,[],{}) is sent to the model as an empty tool message.Model.run_function_callstringified the result only when it was truthy:while
Model.arun_function_callsalready usedstr(function_call.result). Soagent.run()andagent.arun()gave the model different tool results for the same tool, and the model could not tell "zero items" apart from "the tool produced no output". The empty string also ends up inRunOutput.tools[i].result, so it is what gets persisted to the session.This PR drops the truthiness check so the sync branch matches the async one. See the note below on
None.Issue number: #9947
Type of change
Changes
libs/agno/agno/models/base.py: one-line change inrun_function_call,str(function_execution_result.result)unconditionally, matchingarun_function_calls.libs/agno/tests/unit/models/test_tool_result_falsy_values.py: new regression test running a tool that returns0,0.0,False,[],{},1,"text"through bothrun_function_callandarun_function_callswith a stubModel(no network), asserting the tool message content on each path and that the two paths agree.Note on
None: with this change a tool returningNoneproduces"None"on the sync path, which is what the async path has always produced (verified: both paths now give'None'). Previously sync gave"". I kept the two paths identical rather than special-casingNone. If you would rather have""forNone, the change is a one-liner (if result is not None else "") and I'm happy to apply it to both paths.Testing
Commands actually run, in a
uvvenv (uv pip install -e "libs/agno[dev]"plus provider SDKs), onmain@56eae14f:count_items() -> inttool returning0):run()tool message content'',arun()'0'.'0'.0,0.0,False,[],{}and the sync/async equality cases), 10 passed. Restoring the fix: 18 passed.pytest libs/agno/tests/unit/models libs/agno/tests/unit/agent libs/agno/tests/unit/team libs/agno/tests/unit/tools/test_functions.py libs/agno/tests/unit/tools/test_decorator.py libs/agno/tests/unit/tools/test_toolkit.py→ 3121 passed, 12 skipped.pytest libs/agno/tests/unit/workflow→ 560 passed, 7 skipped.ruff format --checkandruff checkon the two changed files → clean.mypy --config-file libs/agno/pyproject.toml libs/agno/agno/models/base.py→ 0 errors.I did not run the entire
libs/agno/tests/unitsuite: some tool modules need optional extras not installed here. CI will cover the rest.Checklist
pyproject.tomlconfig, as./scripts/validate.shdoes)Duplicate and AI-Generated PR Check
Additional Notes
Related open PR: #6633 (for #6361) refactors both branches into a shared
_format_non_generator_resulthelper and, in passing, changes this check tois not None. That PR is much broader (also touches event handling for generators and workflow events), has been conflicting since June, and does not call out the falsy-result behaviour or test it. This PR is the minimal fix for the specific bug in #9947 with a targeted regression test; if #6633 lands first this becomes redundant and can be closed.AI disclosure: not entirely AI-generated. I found and reproduced the bug, decided on the fix, and ran every command listed above; an AI assistant helped draft the test file and this description, and I reviewed and understand every line.