From 6540224f69ec12082b6fdfd887b76b20dfda586b Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Mon, 21 Sep 2026 20:48:51 +0530 Subject: [PATCH] docs: fix three statements the per-model image strip left stale MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The image-rejection recovery no longer switches the session to text-only or strips history; it records the (provider, model) and build_api_request strips images from that model's requests only. Three places still described the old behaviour: - recover_before_classification docstring and the adjacent comment said "switch session to text-only" / "mark session vision-unsupported". - TestStripImagesDropsStaleApiContent's rationale claimed the strip runs on persistent history and that leaving api_content would replay rejected images every turn because the recovery gates on _image_rejecting_models — false on both counts: current callers pass per-call clones. Reworded as the generic helper contract (a rewritten persisted row must drop its sidecar) with the no-op note. - _strip_images_from_messages docstring, same fix. Comments and docstrings only; no code change. --- agent/message_sanitization.py | 4 +++- agent/turn_recovery.py | 12 +++++++----- tests/agent/test_image_rejection_fallback.py | 10 +++++----- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/agent/message_sanitization.py b/agent/message_sanitization.py index c9b0ca6363..48bd81cb55 100644 --- a/agent/message_sanitization.py +++ b/agent/message_sanitization.py @@ -337,7 +337,9 @@ def _strip_images_from_messages(messages: list) -> bool: ``tool`` / ``tool_calls`` messages left empty get a placeholder, NOT deleted (deleting orphans the paired ``tool_call_id`` → HTTP 400); other now-empty messages are dropped. - Rewritten messages lose their ``api_content`` sidecar (it carries the removed images). + Rewritten messages lose their ``api_content`` sidecar (it carries the removed images): + a caller rewriting a persisted row must not leave bytes that replay them next turn. The + current callers pass per-call clones, where this is a no-op. """ from agent.context_compressor import _DB_PERSISTED_MARKER from agent.turn_context import drop_stale_api_content diff --git a/agent/turn_recovery.py b/agent/turn_recovery.py index 3dd48ee67a..4de90f26fd 100644 --- a/agent/turn_recovery.py +++ b/agent/turn_recovery.py @@ -236,9 +236,10 @@ def recover_before_classification( api_kwargs: Any, active_system_prompt: Any, ) -> Tuple[bool, Any]: """Recovery branches that run BEFORE ``classify_api_error``: UnicodeEncodeError - sanitization, provider image-content rejection (switch session to text-only), and the - Bedrock AnthropicBedrock SDK streaming fallback. Returns ``(retry_now, - active_system_prompt)``; the prompt may be ASCII-sanitized in place.""" + sanitization, provider image-content rejection (record the (provider, model); + build_api_request strips images from that model's requests only), and the Bedrock + AnthropicBedrock SDK streaming fallback. Returns ``(retry_now, active_system_prompt)``; + the prompt may be ASCII-sanitized in place.""" if isinstance(api_error, UnicodeEncodeError) and getattr(agent, '_unicode_sanitization_passes', 0) < 2: _recovered, active_system_prompt = _recover_unicode_encode_error( agent, api_error, messages, api_messages, api_kwargs, active_system_prompt @@ -246,8 +247,9 @@ def recover_before_classification( if _recovered: return True, active_system_prompt - # Some providers 4xx on image_url content: strip images, mark session - # vision-unsupported, retry text-only. English phrase match; extend it. + # Some providers 4xx on image_url content: record the (provider, model) and retry; + # build_api_request strips images from that model's requests only. English phrase + # match; extend it. _err_body = "" try: _err_body = str(getattr(api_error, "body", None) or getattr(api_error, "message", None) or str(api_error)) diff --git a/tests/agent/test_image_rejection_fallback.py b/tests/agent/test_image_rejection_fallback.py index d56047620a..59f7bf6ff5 100644 --- a/tests/agent/test_image_rejection_fallback.py +++ b/tests/agent/test_image_rejection_fallback.py @@ -226,14 +226,14 @@ class TestImageRejectionPhraseIsolation: class TestStripImagesDropsStaleApiContent: - """The strip runs on the persistent history, not just the per-call copy. + """Generic helper contract: a rewritten row drops its ``api_content`` sidecar. ``api_content`` is the byte-stability sidecar: it holds the exact bytes previously sent for a message, and the next turn substitutes it back into - ``content``. Leaving it in place on a message this function rewrote would - replay the images the strip just removed — and the recovery cannot re-fire, - because it records the model in ``_image_rejecting_models`` and gates itself - on that. The session would then send rejected images on every subsequent turn. + ``content``. When a caller rewrites a PERSISTED row, the sidecar must go with + it or the next turn replays the images the strip just removed. The current + callers only pass per-call clones (``api_messages``), where dropping the + sidecar is a no-op — the contract is kept for any caller that does not. Same contract the other content-rewrite paths follow (stale-confirmation redaction in ``replay_cleanup``, compression rewrites, merge-into-tail):