Skip to content

fix: handle multimodal content in interim text and avoid retrying local processing errors - #66945

Merged
kshitijk4poor merged 3 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/66341-multimodal-content-retry-loop
Jul 18, 2026
Merged

fix: handle multimodal content in interim text and avoid retrying local processing errors#66945
kshitijk4poor merged 3 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/66341-multimodal-content-retry-loop

Conversation

@kshitijk4poor

@kshitijk4poor kshitijk4poor commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

Hardens multimodal list-content handling in build_assistant_message and _interim_assistant_visible_text, and adds a traceback-based error classifier that stops retrying deterministic local processing errors instead of burning the iteration budget.

Closes #66267
Supersedes #66341 (cherry-picked with authorship preserved)

Changes

  • agent/chat_completion_helpers.py: Use flatten_message_text() in build_assistant_message before the inline domath regex and surrogate sanitizer — protects the non-streaming/gateway path from list content that bypasses the normalization at conversation_loop.py:4387.
  • run_agent.py: Use flatten_message_text() in _interim_assistant_visible_text before _strip_think_blocks — belt-and-suspenders alongside Teknium's fix (296494d) at the shared chokepoint.
  • agent/conversation_loop.py: Traceback-based error classifier — deterministic local processing errors (TypeError from regex on list content) stop immediately instead of retrying until max_iterations. Module sets are module-level frozenset constants; conversation_loop and run_agent are intentionally excluded from _LOCAL_PROCESSING_MODULES because they're container modules that every exception passes through.
  • tests/run_agent/test_66267_multimodal_interim.py: 8 new tests pinning the regression (list content in build_assistant_message, interim text, dedup path).

Salvage notes

  • Removed PR's redundant strip_think_blocks coercion (Teknium's 296494d already handles this with superior logic that drops thinking/reasoning blocks).
  • Restored if not content: return "" guard lost during cherry-pick auto-merge.
  • Fixed docstring corruption (think tags replaced with Chinese characters).
  • Fixed error classifier C1/C2: removed conversation_loop and run_agent from _LOCAL_PROCESSING_MODULES (were making _hit_local always True, misclassifying transient API errors as non-retryable).
  • Replaced traceback.extract_tb() with raw tb walk (avoids disk I/O).

Validation

Tests E2E
Targeted suite 70/70 passed 5/5 real-import checks passed
test_none_returns_empty Fixed (was returning "None") —
Error classifier — Network error → not local; TypeError → local

Attribution

@nanami7777777's commits cherry-picked with authorship preserved. Follow-up fixes by maintainer.

@nanami7777777 @kshitijk4poor
fix: handle multimodal content in interim assistant text and avoid re…
1933004 
…trying local processing errors (NousResearch#66267)
@nanami7777777 @kshitijk4poor
fix: normalize multimodal list content in build_assistant_message (No…
d123815 
…usResearch#66267)

Second call site (non-streaming / gateway path) now flattens list-type
content with flatten_message_text before the inline <think> regex and the
surrogate sanitizer, matching the interim-text fix from the prior commit.

Adds regression tests (tests/run_agent/test_66267_multimodal_interim.py)
covering:
- build_assistant_message with list content does not raise TypeError
- inline <think> inside list content is extracted + stripped correctly
- _interim_assistant_visible_text is safe for tool messages (list content)
- duplicate_previous_interim dedup guards against tool messages

Verified the tests fail without the fix (TypeError: expected string... got
'list') and pass with it.
@kshitijk4poor
kshitijk4poor enabled auto-merge (rebase) July 18, 2026 14:02
@kshitijk4poor
fix: salvage follow-up — remove redundant coercion, fix classifier, r…
6392f50 
…estore None guard

1. Remove PR's redundant strip_think_blocks coercion (Teknium's fix
   296494d already handles this at the same chokepoint with superior
   logic that drops thinking/reasoning blocks).

2. Restore 'if not content: return ' guard at top of strip_think_blocks
   that was lost during cherry-pick auto-merge. Without it, None content
   hits str(None) → 'None' string instead of returning empty.

3. Fix error classifier design flaw: remove 'conversation_loop' and
   'run_agent' from _local_processing_modules — these are the container
   modules for the try/except, so every exception passes through them,
   making _hit_local always True and misclassifying transient API/network
   errors as non-retryable local bugs.

4. Move module sets to module-level frozenset constants (_LOCAL_PROCESSING_MODULES,
   _API_CALL_MODULES) instead of rebuilding on every exception.

5. Replace traceback.extract_tb() with raw tb walk — avoids disk I/O for
   source lines that are never used.

6. Remove unused 'import traceback'.

7. Fix docstring corruption: 3 lines where think tags were replaced
   with Chinese characters during the PR's editing.
@kshitijk4poor
kshitijk4poor force-pushed the salvage/66341-multimodal-content-retry-loop branch from db156c6 to 6392f50 Compare July 18, 2026 14:02
@kshitijk4poor
kshitijk4poor merged commit bd3d16a into NousResearch:main Jul 18, 2026
31 checks passed
@kshitijk4poor kshitijk4poor mentioned this pull request Jul 18, 2026
Closed
@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 tool/vision Vision analysis and image generation P1 High — major feature broken, no workaround labels Jul 18, 2026
This was referenced Jul 18, 2026
Closed
Closed
@teknium1 teknium1 mentioned this pull request Jul 19, 2026
Closed
@Mikoto9901 Mikoto9901 mentioned this pull request Jul 19, 2026
Open
Mikoto9901 added a commit to Mikoto9901/hermes-agent that referenced this pull request Jul 20, 2026
@Mikoto9901
fix(agent): harden structured message text projection
1f385c8 
Structured assistant/tool content (typed parts, multimodal lists,
legacy wrappers) was handled inconsistently across call sites, with
three observable gaps after NousResearch#66267/NousResearch#66945:

* split reasoning tags: flatten_message_text joined parts with '\n',
  so a provider splitting '<think>' across parts ('<thi' + 'nk>...')
  reassembled a tag the scrubber regex could no longer match, leaking
  reasoning into visible text;
* unknown-shape leakage: untyped Mappings with extra provider/tool
  fields, arbitrary objects with .text/.content, and top-level
  str(content) fallbacks could put provider metadata, tool payloads
  or object reprs into the visible reply;
* interim dedup ran the assistant text extractor on any dict message,
  including user/tool/system roles.

Make agent/message_content.py the single canonical projector:

* allow only typed text parts (text/input_text/output_text), explicit
  summary parts, plain strings and PURE legacy wrappers (keys limited
  to text/content); everything else — unknown typed parts, images,
  base64, audio, tool metadata, encrypted/redacted reasoning, unknown
  objects, hostile Mappings whose accessors raise — yields '';
* scrub pipeline (strip_think_blocks, build_assistant_message, interim
  visible text, refusal/length paths) joins parts with sep='' so
  provider-split tags reassemble exactly before scrubbing;
* gate interim dedup on role == 'assistant' before text extraction;
* preserve structured reasoning before projecting assistant content.

Adds tests/run_agent/test_structured_content_projection.py covering
the contract: typed parts, ordering, split <think>, unknown objects,
hostile Mappings, pure wrappers, tool-role guard, refusal/length
normalization.
Mikoto9901 added a commit to Mikoto9901/hermes-agent that referenced this pull request Jul 20, 2026
@Mikoto9901
fix(agent): scope provider terminal lifecycle per attempt
baf0ba3 
Follow-up hardening for NousResearch#66267 / NousResearch#66945 (independent companion to NousResearch#67380,
with no code dependency on it).

The traceback-module classifier cannot reliably tell 'provider request
failed' from 'Hermes-local post-processing failed': once the provider has
delivered a terminal response, any later local exception (aggregation,
usage handling, normalization, even an exception whose type LOOKS like a
network error) could still be routed into provider retry, reconnect,
fallback, continuation, or a partial-stream 'length' stub — re-requesting
an already-completed, billable response. A stale-killed but still-alive
worker can also leak its terminal signal into a newer attempt when phase
lives in shared agent state.

Introduce an explicit, attempt-scoped lifecycle:

* ProviderAttemptLifecycle token (in_flight -> terminal_received) per real
  provider attempt, created by the conversation retry loop and held as a
  loop-local reference.
* Transports capture the token ONCE at the attempt's main entry
  (interruptible_api_call / direct_api_call /
  interruptible_streaming_api_call, on the calling thread before any
  worker starts) and pass it down via explicit parameters or bound
  closures — _dispatch_nonstreaming_api_request, _run_codex_stream,
  _anthropic_messages_create, on_terminal / on_response_received
  callbacks, and the loop's backstop. Nothing past worker start re-reads
  agent._provider_attempt for writer identity.
* Auxiliary entry points invoked without an attempt (e.g. Codex
  iteration-limit summaries, Anthropic auxiliary calls) run on a fresh
  DETACHED token, so they never inherit the main loop's terminal state
  and keep their own internal reconnect.
* Terminal boundary marks: non-streaming raw return on every dispatch
  branch, Chat finish_reason, Anthropic message_stop (incl. shim),
  Codex terminal response event.
* After terminal: no retry / reconnect / fallback / continuation /
  length stub — the turn ends through the unified finalizer as
  local_post_response_error with failed=True and the real api_calls.
* Before terminal: upstream retry/fallback behavior is unchanged; the
  traceback classifier remains only as pre-terminal defense-in-depth.
* Interruption semantics unchanged.

Scope: Chat Completions (streaming/non-streaming), Codex Responses,
Anthropic Messages (streaming/non-streaming, incl. the Kimi Coding
path), DeepSeek chat-completions, and the Bedrock non-streaming
raw-return boundary. Bedrock streaming (converse_stream messageStop)
is explicitly out of scope.

Tests (new, deterministic wire-seam fakes through the real
AIAgent.run_conversation entry):

* test_post_response_retry_boundary.py — non-streaming boundary:
  post-return local error call_count=1, pre-terminal network error
  call_count=2, Bedrock raw-return boundary.
* test_streaming_terminal_boundary.py — finish_reason / message_stop /
  terminal event then local failure: wire call_count=1, no stub, no
  continuation; pre-terminal reconnect preserved; interruption intact.
* test_provider_lifecycle_paths.py — real provider identities and
  routing: Codex Responses, Kimi Coding, Kimi API, DeepSeek, plus
  detached-token auxiliary calls.
* test_provider_attempt_lifecycle.py — deterministic stale-worker
  races (worker alive past stale-kill, and worker delayed before
  dispatch) proving a late terminal only ever lands on its own token.
@kshitijk4poor
kshitijk4poor deleted the salvage/66341-multimodal-content-retry-loop branch August 5, 2026 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P1 High — major feature broken, no workaround tool/vision Vision analysis and image generation type/bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: Multimodal content list crashes interim processing and retries until API-call budget is exhausted

3 participants

@kshitijk4poor @alt-glitch @nanami7777777