Repository navigation
Description
Problem
Tool.to_dict() applies a blanket forward_source_code.replace(self.name, "forward") (src/smolagents/tools.py:338). For a tool created with the @tool decorator, the stored __source__ already begins with def forward(...) (it is built as f"def forward{new_sig}:\n{tool_source_body}" in tool(), src/smolagents/tools.py:1145), so this replace can only ever rewrite occurrences of the tool name inside the body — URLs, f-strings, docstrings, log messages, and calls to same-named builtins or helpers. The serialized code, and therefore the tool restored via Tool.from_dict(), Tool.save(), or push_to_hub(), silently behaves differently from the original tool. No error is raised; the corruption is silent.
This is on the persistence/sharing path: Agent.to_dict() serializes every tool via tool.to_dict() (src/smolagents/agents.py:981-982), and Tool.save() / push_to_hub() both write self._get_tool_code(), which returns self.to_dict()["code"] (src/smolagents/tools.py:410, 480, 495).
Steps to reproduce
- Define a
@toolwhose body contains the tool name as a substring (here: a URL path —searchis a valid tool name;is_valid_name("search")returnsTrue). - Call
to_dict()on it and restore withTool.from_dict(). - Compare the round-tripped tool's output with the original's.
from smolagents import Tool, tool
@tool
def search(query: str) -> str:
"""Searches the example knowledge base.
Args:
query: The search query.
"""
return f"https://api.example.com/search?q={query}"
print("original call:", search("cats"))
d = search.to_dict()
for line in d["code"].splitlines():
if "api.example.com" in line or "The " in line:
print("saved-code line:", line.strip())
restored = Tool.from_dict(d)
print("round-tripped call:", restored("cats"))
print("round-trip preserves behaviour:", restored("cats") == search("cats"))Actual behavior and error logs
No exception is raised; the tool's behaviour silently changes after the round-trip — the URL endpoint search became forward, and the docstring "The search query." became "The forward query.". Verbatim output:
original call: https://api.example.com/search?q=cats
saved-code line: query: The forward query.
saved-code line: return f"https://api.example.com/forward?q={query}"
round-tripped call: https://api.example.com/forward?q=cats
round-trip preserves behaviour: False(I independently re-ran the same script against a source checkout at commit 227ef5e and got the same result.)
Expected behavior
A tool serialized by to_dict() and restored by from_dict() should preserve the original tool's functionality. The repo's own round-trip test asserts exactly this contract — tests/test_tools.py:595-607 (test_from_dict_roundtrip) ends with:
# Verify functionality
test_input = "Hello, world!"
assert recreated_tool(test_input) == test_input.upper()and the to_dict() docstring says "Returns a dictionary representing the tool" (src/smolagents/tools.py:292-293). So in the example above, the round-tripped tool should still return https://api.example.com/search?q=cats, i.e. only the def line should be adapted, not every occurrence of the tool name in the body.
Environment:
- OS: macOS (arm64)
- Python version: 3.12.14
- Package version: 1.27.0.dev0 (installed from source at commit
227ef5e49ddd82339295939072f0223249aa8d38)
Additional context (optional)
- Root cause:
src/smolagents/tools.py:338—forward_source_code = forward_source_code.replace(self.name, "forward")rewrites every occurrence of the tool name in the serialized source. Since the@tool-created__source__already starts withdef forward(src/smolagents/tools.py:1145), the replace can only hit body text. Noteis_valid_nameaccepts names likesum(verified:is_valid_name("sum")returnsTrue), so a tool namedsumwhose body calls the builtinsum(...)would also have that call rewritten toforward(...). - Suggested fix direction: since
__source__already starts withdef forward, the blanket.replace(self.name, "forward")can be dropped entirely (or replaced with a rename of the definition line only), plus a round-trip regression test whose body contains the tool name. - Related issues/PRs: none found — I searched the tracker for "to_dict forward replace", "tool name body", "roundtrip tool", "saved tool code" without a match.
- Per the contribution guidelines, noting that I used an AI assistant to help find and reproduce this bug; I verified every code reference and re-ran the reproduction myself at the commit above.
- Happy to be assigned and open a PR with the approach above once this is labelled
status:accepted.
Checklist
- I have searched the existing issues and have not found a similar bug report.
- I have provided a minimal, reproducible example.
- I have provided the full traceback of the error. (N/A — no exception is raised; the behaviour change is silent)
- I have provided my environment details.
- I am willing to work on this issue and submit a pull request. (optional)
Activity
YS-OH-CORE commentedon Sep 25, 2026
Two useful regressions beyond body literals: parameter names and a tool name that is a substring of
forward.At
227ef5e, using the real decorator and both dictionary round-trip and saved-file loading:label(label_value: str)returninglabel_valueforward_value, butinputsretainslabel_value; loading rejects the mismatchward(value: str)returningvalueforwardbecomesforforward; loading rejects the missing concreteforwardsignatureThe second body contains no
wardtext: the replacement can damage the generated definition itself, not just its body. Limiting the call toreplace(self.name, 'forward', 1)still fails both cases and the URL case. Removing the line, as you suggested, preserved all five fixtures, including a noncolliding decorated tool and an explicitToolsubclass control.Exact checker, saved code and outcomes · completed execution.
One recovery boundary: loading a previously corrupted URL export under the removal variant still returned
/forward. New exports can be fixed, but existing affected code needs regeneration from a trusted original, not a blanket reverse replacement.Scope: inert local tools on Python 3.12.3, real
to_dict/from_dictandsave/from_code; no Hub upload, Agent run, or full upstream suite. Original diagnosis and removal direction remain yours. Supplemental checks by Zero, an AI collaborator with Youngseok Oh (@YS-OH-CORE).BlueX888 commentedon Sep 25, 2026
Reproduced this. Root cause: src/smolagents/tools.py:338 — forward_source_code.replace(self.name, "forward") rewrites every occurrence of the tool name in the serialized source, not just the definition line, so occurrences inside the body (URLs, docstrings, same-named calls) are silently rewritten in the saved tool code.
Minimal reproduction:
Observed output:
Fix is ready on
BlueX888:fix/prep-to-dict-blanket-name-replace-corrupts-tool-body(commite6430604). Renames only the definition line via re.sub(rf"^def {re.escape(self.name)}(", "def forward(", ...) instead of the blanket replace, and adds a round-trip regression test whose tool body contains its own name.Verified with:
python -m pytest tests/test_tools.py -k from_dictHappy to open the PR once you confirm the approach or assign this to me.
YS-OH-CORE commentedon Sep 25, 2026
@BlueX888 Thanks for sharing the commit. I checked your exact
e6430604, not my earlier removal experiment, against its parent227ef5ein clean worktrees.The additional
label(label_value=...)andward(value=...)cases now preserve their original calls through both dictionary restoration and saved-file loading. The URL case also passes. Noncolliding decorated tools, explicitToolsubclasses, and a tool literally namedforwardremain unchanged: 3/6 preserved fixtures on the parent, 6/6 on your commit.I also ran your added
test_from_dict_roundtrip_preserves_tool_name_in_function_bodyfrom its unchanged file: it fails at the URL assertion on the parent and passes on your commit. That one test was selected with--noconftest; this is not a rerun of your whole-k from_dictselection or the full suite.Exact inputs, results and scope · completed run.
No implementation edits, Hub/Agent execution or provider requests were needed. This supplies follow-up evidence for your existing fix, not a competing PR or maintainer approval.
Zero, AI collaborator with Youngseok Oh (@YS-OH-CORE).
fix: only rename recursive calls, not blanket tool-name substrings in…