Skip to content

fix(agent): recover tools from truncated scratchpads - #64306

Open
embwl0x wants to merge 1 commit into
NousResearch:mainfrom
embwl0x:agent/recover-truncated-scratchpad-tool-calls
Open

fix(agent): recover tools from truncated scratchpads#64306
embwl0x wants to merge 1 commit into
NousResearch:mainfrom
embwl0x:agent/recover-truncated-scratchpad-tool-calls

Conversation

@embwl0x

@embwl0x embwl0x commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • preserve provider-emitted structured tool calls when adjacent <REASONING_SCRATCHPAD> text is truncated
  • remove the unclosed reasoning tail before the assistant turn is persisted or replayed, while retaining any visible prefix
  • route recovered calls through the existing tool-name, JSON-object, scope, guardrail, and approval paths
  • keep the existing two-retry / partial-failure behavior for text-only incomplete scratchpads

Root cause

run_conversation() checked the free-form scratchpad before its structured assistant_message.tool_calls. Any unclosed scratchpad caused an unconditional retry and discarded the whole response, even when the provider had already returned a complete, independently structured tool call. After the retry, the useful action was gone.

The recovery is intentionally narrow: only a non-empty structured tool-call list bypasses the scratchpad retry. Hermes strips the unclosed text tail and then enters its normal tool pipeline. Invalid names and arguments are still rejected there, and tools still require the same configured approvals.

Current-main proof

An isolated mocked response with an unclosed scratchpad plus a complete web_search({}) call produced:

{'final_response': 'Done', 'api_calls': 2, 'tool_calls_executed': 0}

After this change, the same proof produces:

{'final_response': 'Done', 'api_calls': 2, 'tool_calls_executed': 1}

The follow-up request contains the tool call and result but no unclosed REASONING_SCRATCHPAD text.

Validation

  • isolated mocked-provider before/after proof under a temporary HERMES_HOME
  • uv run --extra dev pytest tests/run_agent/test_run_agent.py::TestRunConversation -q (53 passed)
  • uv run --extra dev pytest tests/run_agent/test_run_agent.py -q (425 passed)
  • uv run --extra dev ruff check agent/conversation_loop.py tests/run_agent/test_run_agent.py
  • git diff --check
  • Hermes contribution publish gate, overlap adjudication, metadata scan, and gitleaks

Overlap audit

PR #11743 handles false positives from quoted/code-form scratchpad tag mentions. The other open REASONING_SCRATCHPAD matches affect title sanitization, UI token normalization, trajectory coverage, or list-valued session logging. None handles a true incomplete scratchpad response carrying structured tool calls.

Fixes #11153

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists labels Jul 14, 2026
@embwl0x

embwl0x commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

CI follow-up: the only failing slice is tests/hermes_cli/test_model_validation.py, with the same two Gemini probe user-agent tests still patching urllib.request.urlopen after production moved to _urlopen_model_catalog_request.

I reproduced both failures on an untouched checkout at this PR's exact base, 226e8de827a669e8ffa7035b27d70c19e44b1208; this PR does not touch the model-probe path, and its complete tests/run_agent/test_run_agent.py surface passes locally (425 tests) and in the CI slice. The focused upstream correction is already open, fully green, and mergeable in #64200. I am keeping this recovery fix scoped to #11153 and will refresh CI after that baseline correction lands.

@embwl0x
embwl0x force-pushed the agent/recover-truncated-scratchpad-tool-calls branch from 8144cb6 to bd63dc1 Compare July 14, 2026 18:45
@embwl0x

embwl0x commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Refreshed onto current main (1f41bdbecd) in bd63dc1cae now that the focused model-probe baseline correction has landed.

The branch rebased without conflicts. Current-head local verification passes 517 tests across the complete run-agent suite and model-validation suite; Ruff, diff check, public identity, overlap review, metadata scan, and gitleaks also pass. Replacement GitHub CI is running on the refreshed head.

@embwl0x
embwl0x force-pushed the agent/recover-truncated-scratchpad-tool-calls branch from bd63dc1 to 1d08c51 Compare July 15, 2026 08:56
@embwl0x

embwl0x commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (569b912d7d) in 1d08c517c6 after the overnight Codex continuation changes made the branch conflict.

The production change composed cleanly. The only manual conflict was the test insertion point, where current main added null-tool-argument coverage; the refreshed branch preserves that regression alongside all three incomplete-scratchpad recovery cases.

Current-head verification: the complete tests/run_agent/test_run_agent.py file passes (433 passed), including the newly landed Codex continuation tests. Ruff, Windows-footgun scan, uv lock --check, diff check, current-main ancestry, overlap review, public identity, metadata scan, and gitleaks also pass. Replacement GitHub CI is running.

@embwl0x

embwl0x commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

CI slice 8 is currently red on test_normalize_codex_response_salvage_is_xai_scoped, but this is a deterministic regression on the exact base commit 569b912d7, not a failure introduced by this PR.

I reproduced the same result in an untouched origin/main worktree:

scripts/run_tests.sh tests/agent/test_codex_responses_adapter.py
25 passed, 1 failed

The focused test-only corrections are #64836 and #64844; both are green. I am keeping this PR scoped to the scratchpad recovery fix and will rebase/revalidate once that base correction lands.

@embwl0x
embwl0x force-pushed the agent/recover-truncated-scratchpad-tool-calls branch from 1d08c51 to f2dcf69 Compare July 15, 2026 15:41
@embwl0x

embwl0x commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Refreshed onto current main after #64844 merged.

New head: f2dcf69dc

Validation:

  • scripts/run_tests.sh tests/run_agent/test_run_agent.py tests/agent/test_codex_responses_adapter.py -> 459 passed
  • the formerly failing test_normalize_codex_response_salvage_is_xai_scoped now passes
  • ruff, uv lock --check, git diff --check, public-identity/privacy checks, gitleaks, and the existing-PR publish gate all pass

The branch is current-main-based, ready for review, and GitHub reports it mergeable.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused recovery fix. Current main unconditionally retries incomplete scratchpads at agent/conversation_loop.py:4446-4454 before structured tool processing starts at agent/conversation_loop.py:4585; that confirms the reported loss of complete structured calls. The change in f2dcf69dcf00e763771c6f5fbf26de221c45e0e5 removes only the unclosed text tail and then reuses the existing name, JSON, and guardrail paths at agent/conversation_loop.py:4596-4769. The three added regressions cover the recovered valid call, rejected non-object arguments, and retained text-only retry behavior. GitHub reports all required checks passing.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 16, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

One PR, #64306, addresses issue #11153 by changing incomplete-scratchpad handling so independently structured tool calls survive truncation while text-only truncation retains the bounded retry and partial-failure behavior.

Related pull requests

  • fix(agent): recover tools from truncated scratchpads #64306 best fix — (+115/-22) — n/a: The diff strips only the unclosed reasoning tail when structured tool calls are present, then routes those calls through the existing validation and approval pipeline; three regressions cover valid-call recovery, rejection of non-object arguments, and unchanged text-only retry behavior.

Suggested consolidation

Keep #64306 open with a salvage path, consistent with the automated keep-open review: preserve its narrowly scoped structured-call recovery and three regression tests, with author action to rebase onto main if needed. No competing PR or duplicate is present in this complex.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I11153(["issue #11153 (open)"])
    P64306["PR #64306 (open)"]
    P64306 -->|best fix| I11153
    class I11153 open
    class P64306 open
    class P64306 best
    class P64306 target
    click I11153 "https://github.com/NousResearch/hermes-agent/issues/11153"
    click P64306 "https://github.com/NousResearch/hermes-agent/pull/64306"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 8 kB of PR diffs, 5 kB of issue/PR text, 3 kB of discussion (6 comments), 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

@alt-glitch alt-glitch removed the sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) label Aug 4, 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-broad Sweeper blast radius: broad — a core path most sessions hit 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.

[Resilience] Frequent incomplete <REASONING_SCRATCHPAD> causes wasted retries and lost partial output

4 participants