Skip to content

BUG: Tool.to_dict() blanket replace of the tool name rewrites occurrences inside the saved forward body, silently changing round-tripped behaviour #2833

Open

Description

@BlueX888

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

  1. Define a @tool whose body contains the tool name as a substring (here: a URL path — search is a valid tool name; is_valid_name("search") returns True).
  2. Call to_dict() on it and restore with Tool.from_dict().
  3. 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 with def forward (src/smolagents/tools.py:1145), the replace can only hit body text. Note is_valid_name accepts names like sum (verified: is_valid_name("sum") returns True), so a tool named sum whose body calls the builtin sum(...) would also have that call rewritten to forward(...).
  • Suggested fix direction: since __source__ already starts with def 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

  1. YS-OH-CORE commented on Sep 25, 2026

    @YS-OH-CORE

    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:

    Inert decorated tool Serialized result
    label(label_value: str) returning label_value Parameter becomes forward_value, but inputs retains label_value; loading rejects the mismatch
    ward(value: str) returning value Generated method forward becomes forforward; loading rejects the missing concrete forward signature

    The second body contains no ward text: the replacement can damage the generated definition itself, not just its body. Limiting the call to replace(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 explicit Tool subclass 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_dict and save/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).

  2. BlueX888 commented on Sep 25, 2026

    @BlueX888
    Author

    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:

    python - <<'PY'
    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"))
    PY
    

    Observed 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
    

    Fix is ready on BlueX888:fix/prep-to-dict-blanket-name-replace-corrupts-tool-body (commit e6430604). 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_dict

    Happy to open the PR once you confirm the approach or assign this to me.

  3. YS-OH-CORE commented on Sep 25, 2026

    @YS-OH-CORE

    @BlueX888 Thanks for sharing the commit. I checked your exact e6430604, not my earlier removal experiment, against its parent 227ef5e in clean worktrees.

    The additional label(label_value=...) and ward(value=...) cases now preserve their original calls through both dictionary restoration and saved-file loading. The URL case also passes. Noncolliding decorated tools, explicit Tool subclasses, and a tool literally named forward remain 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_body from 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_dict selection 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).

  4. added a commit that references this issue on Sep 25, 2026
    c21cbf7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions