Skip to content

fix(agent): deepcopy agent.tools before llama.cpp grammar-retry sanitization - #34595

Open
AhmetArif0 wants to merge 1 commit into
NousResearch:mainfrom
AhmetArif0:fix/llama-cpp-grammar-deepcopy
Open

fix(agent): deepcopy agent.tools before llama.cpp grammar-retry sanitization#34595
AhmetArif0 wants to merge 1 commit into
NousResearch:mainfrom
AhmetArif0:fix/llama-cpp-grammar-deepcopy

Conversation

@AhmetArif0

Copy link
Copy Markdown
Contributor

Root Cause

strip_pattern_and_format() is documented to mutate its input in place:

Callers that need to preserve the original should deep-copy first.
tools/schema_sanitizer.py

The llama.cpp grammar-retry path in conversation_loop.py called it directly
on agent.tools without a deep-copy, permanently stripping pattern/format
keywords from the shared tool-registry dicts — not only for the immediate
retry, but for every subsequent API call in the same session.

Affected sessions: any that mix a llama.cpp primary model with cloud-provider
auxiliary tasks (Anthropic, OpenAI, OpenRouter, Gemini), or that switch models
mid-session after a grammar error. Those follow-on calls lose the schema hints
that cloud providers rely on for date/phone/email parameters.

Missed sibling of #34416

PR #34416 applied the identical deep-copy fix to
chat_completion_helpers.py (line 623) and auxiliary_client.py (line 708)
after identifying that the xAI sanitizer paths suffered the same permanent
mutation. This call site in conversation_loop.py was not included.

Fix

# before
_, _stripped = strip_pattern_and_format(agent.tools)

# after
_tools_copy = _copy.deepcopy(agent.tools)
_, _stripped = strip_pattern_and_format(_tools_copy)
agent.tools = _tools_copy
  • Registry dicts are preserved; a future /reload-mcp or tool rebuild returns
    full schemas.
  • The session-level permanent strip (so subsequent llama.cpp calls don't
    re-trigger the retry) is retained via the explicit agent.tools = _tools_copy
    assignment — behaviour is unchanged for pure llama.cpp sessions.
  • The exception path is safer: a partial failure in strip_pattern_and_format
    no longer leaves agent.tools in a partially-mutated state.

Test plan

  • Start a session with a llama.cpp OAI-compatible server that rejects
    pattern/format in tool schemas (HTTP 400 grammar error).
  • Confirm the grammar-retry fires and the session continues.
  • Switch to a cloud provider (or trigger an auxiliary task routed to one)
    in the same session — verify the tool schemas sent to the cloud provider
    still contain pattern/format keywords.
  • Existing test_run_agent_codex_responses.py and schema-sanitizer unit
    tests pass unchanged.

…ization

strip_pattern_and_format() mutates its input in place (documented in
schema_sanitizer.py: "Callers that need to preserve the original should
deep-copy first"). The llama.cpp grammar-retry path called it directly
on agent.tools, permanently stripping pattern/format keywords from the
shared tool-registry dicts — not just for this retry, but for every
subsequent call in the session (auxiliary tasks routed to a cloud
provider, model switches).

PR NousResearch#34416 applied the same deepcopy fix to chat_completion_helpers.py
and auxiliary_client.py but this call site was missed.

Fix: deepcopy agent.tools into _tools_copy, strip from the copy, then
assign back. The registry dicts are preserved; future tool rebuilds from
the registry return full schemas. The session-level permanent strip for
llama.cpp (so subsequent llama.cpp calls don't re-trigger the retry) is
retained via the explicit agent.tools = _tools_copy assignment.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels May 29, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Missed sibling of #34416 — same deepcopy fix for strip_pattern_and_format() mutation, this time in the conversation_loop.py llama.cpp grammar-retry path (the other two call sites were fixed in #34416).

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for isolating the remaining in-place sanitizer call. Current main still invokes strip_pattern_and_format(agent.tools) directly at agent/conversation_loop.py:2946, while the helper documents its in-place contract at tools/schema_sanitizer.py:382-388. This is particularly consequential because the quiet-mode tool-definition cache shares schema-dict references (model_tools.py:327-335).

Problems

  • The PR has no regression coverage for this recovery branch. Current tests cover classifier recognition and sanitizer behavior, but not preservation of the source schema after llama.cpp recovery.

Suggested changes

  • Add a focused test that drives the grammar recovery, confirms the retry schema is stripped, and confirms the original/cached tool schema retains pattern and format.

The code change itself matches the prior deepcopy pattern in 1386a7e4789c9b886395804e8475a4252217e4ac; current surrounding code has moved to agent/conversation_loop.py, so salvage should be a small conflict-resolution cherry-pick. Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
@mvdbastos

Copy link
Copy Markdown

Flagging for visibility: we're pulling #67349 into our fork, which touches this same strip_pattern_and_format(agent.tools) call site in conversation_loop.py. Worth landing this deepcopy fix alongside/before it so the two don't conflict — #67349's retry path would otherwise inherit the same shared-dict mutation bug this PR fixes. Happy to help rebase either one on top of the other if useful.

@alt-glitch alt-glitch removed the sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades label Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants