Skip to content

refactor(sanitizer): extract shared _classify_tool_call_orphans to eliminate drift - #59434

Closed
isheng-eqi wants to merge 3 commits into
NousResearch:mainfrom
isheng-eqi:refactor/dedup-orphan-tool-pair-sanitizer
Closed

isheng-eqi wants to merge 3 commits into
NousResearch:mainfrom
isheng-eqi:refactor/dedup-orphan-tool-pair-sanitizer

Conversation

@isheng-eqi

Copy link
Copy Markdown
Contributor

What

sanitize_api_messages in agent_runtime_helpers.py and _sanitize_tool_pairs in context_compressor.py had near-identical orphan-detection logic (collect tool-call IDs from assistant tool_calls and tool messages, then classify orphans). The two copies had already drifted: the canonical sanitizer gained dedup in #58350, but the compressor copy did not.

Fix

Extract the shared orphan-classification logic into _classify_tool_call_orphans(messages) in agent_runtime_helpers.py. Both call sites now delegate to it for ID collection and orphan detection, preserving their divergent remediation strategies (insert-stubs vs strip-orphans) while ensuring id-resolution rules (call_id || id) and future dedup additions never drift again.

Changes

  • agent/agent_runtime_helpers.py: +23 −13 — new _classify_tool_call_orphans() function; sanitize_api_messages delegates orphan detection to it
  • agent/context_compressor.py: +21 −21 — _sanitize_tool_pairs delegates to the shared function instead of reimplementing ID collection

Test plan

All 58 tests in tests/run_agent/test_413_compression.py, test_860_dedup.py, and test_anthropic_prompt_cache_policy.py pass — the compression, dedup, and prompt-caching behaviours are unchanged.

Related

Closes #58357

…iminate drift

sanitize_api_messages (agent_runtime_helpers) and
_sanitize_tool_pairs (context_compressor) both collected
tool-call IDs and classified orphans with near-identical logic
that had already drifted: the canonical sanitizer added dedup
(NousResearch#58350), but the compressor's copy did not.

Extract the shared orphan-detection logic into
_classify_tool_call_orphans(messages) in agent_runtime_helpers.
Both call sites now delegate to it, preserving their divergent
remediation strategies (insert-stubs vs strip-orphans) while
ensuring id-resolution rules and dedup stay in sync.

Closes NousResearch#58357
@alt-glitch alt-glitch added type/refactor Code restructuring, no behavior change comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have labels Jul 6, 2026
@isheng-eqi

Copy link
Copy Markdown
Contributor Author

@kshitijk4poor @alt-glitch This addresses #58357 — preventing the same drift that caused #55845 and #58327. Already has 5 unit tests, all compression/dedup suites pass. Ready for review when you have time.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for consolidating the duplicated orphan-classification path; current main still has the two implementations (agent/agent_runtime_helpers.py:2591-2604 and agent/context_compressor.py:2507-2520).

Problems

  • The new classifier in ea37ed48d0f0 uses AIAgent._get_tool_call_id_static, which strips IDs (run_agent.py:3750-3754), but compressor remediation still calls its unstripped local _get_tool_call_id (agent/context_compressor.py:2479-2483, :2544). A whitespace-padded missing call_id is classified as missing under the normalized value but will not be removed by the raw-value filter.
  • The extraction does not consolidate deduplication: sanitize_api_messages still owns the only duplicate-ID pass (agent/agent_runtime_helpers.py:2640-2682), while the shared classifier only produces sets. That does not yet meet the stated “dedup additions never drift” goal.

Suggested changes

  • Share one normalized ID extractor across classification and compressor stripping, and add a compressor integration test covering whitespace plus a missing result.
  • Either share the duplicate-ID invariant too, or scope the PR description/docstring to orphan classification.

Automated hermes-sweeper review.

@teknium1

Copy link
Copy Markdown
Collaborator

Merged via PR #97167 (#97167) — your commits cherry-picked onto current main with authorship preserved in git log. The shared _classify_tool_call_orphans was reconciled to main's newer alias-aware variant-set semantics so both call sites now delegate to one classifier with byte-identical behavior — exactly the drift elimination you were after. (The release.py hunk was dropped; contributor mapping moved to per-email files.) Thanks!

@teknium1 teknium1 closed this Aug 28, 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 P3 Low — cosmetic, nice to have 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/refactor Code restructuring, no behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(compression): consolidate _sanitize_tool_pairs with the canonical agent_runtime_helpers sanitizers (dedup/whitespace/orphan drift)

3 participants