Skip to content

fix(agent): persist image-shrink recovery - #62005

Open
embwl0x wants to merge 5 commits into
NousResearch:mainfrom
embwl0x:agent/persist-image-shrink
Open

fix(agent): persist image-shrink recovery#62005
embwl0x wants to merge 5 commits into
NousResearch:mainfrom
embwl0x:agent/persist-image-shrink

Conversation

@embwl0x

@embwl0x embwl0x commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • record the exact data-URL replacements produced by reactive image-shrink recovery
  • mirror those replacements into canonical session history for OpenAI and Anthropic image shapes
  • persist the repaired history before retrying, preventing later turns from re-sending and re-encoding the same oversized images
  • avoid a second decode/re-encode by reusing the already successful provider-facing repair

Current-main proof

On current main, try_shrink_image_parts_in_messages() mutates only the provider-facing api_messages copy. The new regression failed before the fix because the helper could not expose any replacement for canonical history; the original oversized payload therefore remained in messages for the next turn.

After the fix, the helper returns the exact old-to-new payload mapping, detached canonical OpenAI and Anthropic messages are rewritten, and the repaired session is persisted before the retry. The retry-loop regression records the first rejected request, repaired canonical persistence, and repaired retry in order while asserting the image encoder runs exactly once.

Duplicate / sibling-path audit

Reviewed open image-shrink PRs #37412, #42509, #49124, #52444, and #53884. They cover 413 routing, classification, downscale acceptance, or wrapped-error parsing. None persists a successful repair into canonical session history.

Sibling shapes audited: OpenAI image_url dict/string parts and Anthropic native base64 image.source blocks.

Verification

  • tests/run_agent/test_image_shrink_recovery.py: 25 passed
  • 413 and image-rejection suites: 42 passed
  • image routing, compression persistence, and retry-state suites: 109 passed
  • tests/run_agent/test_run_agent.py: 416 passed
  • Ruff passed on all changed files
  • git diff --check passed
  • pushed-range gitleaks passed

No live Hermes state or provider credentials were used.

Fixes #61994

@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 sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 10, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing the recovery state through canonical session persistence. The premise is confirmed on current main: Anthropic cache preparation deep-copies the API request (agent/prompt_caching.py:84-99), while the image-too-large retry only shrinks api_messages (agent/conversation_loop.py:2632-2647).

Problems

  • The new loop-level persistence behavior lacks a regression test. tests/run_agent/test_image_shrink_recovery.py:12-15 explicitly documents that retry-loop wiring is not tested, and the added tests only validate mapping collection and standalone canonical replacement. They do not assert the new _persist_session() call in the retry path.

Suggested changes

  • Add a focused retry-path test with detached canonical and API message objects. Assert the canonical payload is rewritten, _persist_session(messages, conversation_history) is called before retry, and the retry uses the repaired image without another encode.

Automated hermes-sweeper review.

@embwl0x

embwl0x commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

Addressed in 31ae7be on current main (a0a6cd80). The new retry-loop regression uses detached canonical/API image objects and asserts the order first rejected request -> canonical persistence -> repaired retry, with exactly one resize call (25 focused tests pass). The branch update is published on the earlier #62005 PR; no new PR was opened. The later competing #62051 remains documented for maintainer selection, and its sweeper review points to the reactive persistence design in #62005 rather than a global ingestion cap.

@embwl0x
embwl0x force-pushed the agent/persist-image-shrink branch from a82bfca to c4982dd Compare July 11, 2026 12:36
@embwl0x
embwl0x force-pushed the agent/persist-image-shrink branch from c4982dd to 31ae7be Compare July 11, 2026 12:44
@teknium1 teknium1 added sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 11, 2026
@embwl0x

embwl0x commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Resolved the current-main conflict in e76f996 while preserving both invariants: image-shrink remains copy-on-write for API-local messages, and a confirmed provider rejection still mirrors the exact successful repair into canonical history before persistence and retry.

Local verification:

  • 27 focused image-shrink tests passed
  • 185 adjacent image routing, prompt caching, retry-state, and compression persistence tests passed
  • 446 run-agent tests passed with isolated HERMES_HOME
  • Ruff, git diff --check, publish gate, and gitleaks passed

GitHub now reports the PR mergeable; replacement CI is running.

…t-20260730

# Conflicts:
#	tests/run_agent/test_image_shrink_recovery.py
@embwl0x

embwl0x commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Resolved the new current-main conflict in 63e5b4d56 while preserving both image-shrink invariants: API-local rewriting remains copy-on-write, and a confirmed provider rejection still mirrors the exact successful repair into canonical history before persistence and retry.

The conflict was in tests/run_agent/test_image_shrink_recovery.py. I retained the PR's persistence regressions and current main's stronger copy-on-write/dimension coverage, without resurrecting the redundant dimension test removed upstream.

Local verification:

  • 16 focused image-shrink recovery tests passed
  • 73 adjacent image routing, prompt caching, retry-state, and compression persistence tests passed
  • 220 full run-agent tests passed in an isolated HERMES_HOME
  • Ruff, Python compilation, git diff --check, publish gate, and PR-only gitleaks passed

GitHub reports the updated head mergeable; replacement CI is running.

@embwl0x

embwl0x commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Replacement CI completed with the conflict resolution itself clean: 7/8 Python slices passed, both Docker builds passed, Desktop E2E passed, and all lint/security/lockfile checks passed.

Slice 8 failed only in tests/tools/test_vercel_sandbox_environment.py: 16 tests hit FeatureUnavailable because terminal.vercel is absent and lazy installs are disabled. This is an upstream-base failure introduced by the current-main Vercel restoration, not by this PR:

  • Exact base tested: 2d404942471633d5338a8ff514ea7da24549274f
  • Untouched detached base worktree: the same 16 tests fail with the same FeatureUnavailable path
  • Updated PR head: the same 16 tests fail
  • The PR diff remains limited to the four image-shrink files

I did not import an unrelated Vercel fixture repair into this image-shrink PR. The conflict remains resolved and GitHub reports the head mergeable.

@embwl0x

embwl0x commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Upstream landed the unrelated Vercel CI fixture correction in 8eb06e75b. I merged current origin/main into this branch at be96ff449, preserving the image-shrink diff without additional source changes.

Verification:

  • 16 focused image-shrink recovery tests passed locally
  • 16 Vercel sandbox tests that previously failed passed locally
  • focused Ruff, git diff --check, publish gate, and gitleaks passed
  • replacement CI is fully green: all 8 Python slices, both Docker builds, Desktop E2E, lint, security, supply-chain, and the required aggregate gate

GitHub reports the refreshed head mergeable.

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-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) 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.

image-shrink recovery re-encodes oversized images on every API call — repaired images never persisted to session history

3 participants