Fix two chat-history bugs in gr.ChatInterface and gr.load_chat

.changeset/great-pears-share.md

@@ -0,0 +1,5 @@
+---
+"gradio": patch
+---
+
+fix:Fix two chat-history bugs in `gr.ChatInterface` and `gr.load_chat`

gradio/chat_interface.py

@@ -925,7 +925,10 @@ async def _submit_fn(
         history: list[MessageDict],
         *args,
     ) -> tuple:
-        inputs = [message, history] + list(args)
+        # `list(history)` so that appending to it inside the chat function does not
+        # change the conversation. Shallow, matching `_append_message_to_history`, so
+        # editing one of the messages in place still shows through. See #10823.
+        inputs = [message, list(history)] + list(args)
         if self.is_async:
             response = await self.fn(*inputs)
         else:
@@ -949,7 +952,9 @@ async def _stream_fn(
         tuple,
         None,
     ]:
-        inputs = [message, history] + list(args)
+        # `list(history)` for the same reason as in `_submit_fn`: appending to it in the
+        # generator's body must not be able to change the conversation. See #10823.
+        inputs = [message, list(history)] + list(args)
         if self.is_async:
             generator = self.fn(*inputs)
         else:

gradio/external.py

@@ -748,6 +748,25 @@ def fn(*data):
 IMAGE_FILE_EXTENSIONS = (".png", ".jpg", ".jpeg", ".gif", ".webp")
 
 
+_TEXT_FILE_EXTENSIONS_LOWERCASE = tuple(ext.lower() for ext in TEXT_FILE_EXTENSIONS)
+
+
+def _is_text_encoded_file(path: str) -> bool:
+    # Matched case-insensitively on both sides, the way `MultimodalTextbox` matches
+    # `file_types`, so that the `.R` and `.Rmd` entries above cover `plot.r` too.
+    return path.lower().endswith(_TEXT_FILE_EXTENSIONS_LOWERCASE)
+
+
+def _text_encoded_file_as_prompt(path: str) -> str:
+    if is_http_url_like(path):
+        response = httpx.get(path)
+        response.raise_for_status()
+        name, contents = Path(httpx.URL(path).path).name, response.text
+    else:
+        name, contents = Path(path).name, Path(path).read_text()
+    return f"\n## {name}\n{contents}"
+
+
 def format_conversation(
     history: list[NormalizedMessageDict], new_message: str | MultimodalValue
 ) -> list[dict]:
@@ -760,18 +779,28 @@ def format_conversation(
                     f"Invalid message format: {message['content']}. Each element must have a type key."
                 )
             elif content["type"] == "file":
-                new_content.append(
-                    {
-                        "type": "image_url",
-                        "image_url": {
-                            "url": encode_url_or_file_to_base64(content["file"]["path"])  # type: ignore
-                        },
-                    }
-                )
+                path = content["file"]["path"]  # type: ignore
+                if _is_text_encoded_file(path):
+                    # Appended to the prompt as text, the same as when the message was
+                    # first sent. Base64-encoding a text file as `image_url` (which is
+                    # what used to happen once the turn was in the history) makes a
+                    # non-multimodal model reject every subsequent request. See #11331.
+                    new_content.append(
+                        {"type": "text", "text": _text_encoded_file_as_prompt(path)}
+                    )
+                else:
+                    new_content.append(
+                        {
+                            "type": "image_url",
+                            "image_url": {"url": encode_url_or_file_to_base64(path)},
+                        }
+                    )
             else:
                 new_content.append(content)
-        message["content"] = new_content
-        conversation.append(message)
+        # A new dict rather than `message["content"] = new_content`: these are the dicts
+        # in `chatbot_state`, and rewriting them there replaces the attachment shown in
+        # the transcript (and saved by `save_history`) with what was sent to the model.
+        conversation.append({**message, "content": new_content})
     if isinstance(new_message, str):
         text = new_message
         files = []
@@ -780,7 +809,7 @@ def format_conversation(
         files = new_message.get("files", [])
     image_files, text_encoded = [], []
     for file in files:
-        if file.lower().endswith(TEXT_FILE_EXTENSIONS):
+        if _is_text_encoded_file(file):
             text_encoded.append(file)
         else:
             image_files.append(file)
@@ -799,12 +828,7 @@ def format_conversation(
         )
     if text or text_encoded:
         text = text or ""
-        text += "\n".join(
-            [
-                f"\n## {Path(file).name}\n{Path(file).read_text()}"
-                for file in text_encoded
-            ]
-        )
+        text += "\n".join([_text_encoded_file_as_prompt(file) for file in text_encoded])
         conversation.append(
             {"role": "user", "content": [{"type": "text", "text": text}]}
         )

test/test_chat_interface.py

@@ -278,6 +278,37 @@ def test_example_caching_with_additional_inputs_already_rendered(
                 Message(role="assistant", content=[TextMessage(text="ro")]),
             ]
 
+    @pytest.mark.asyncio
+    async def test_history_mutation_in_fn_is_ignored_consistently(self):
+        # Mutating `history` inside the chat function used to change the conversation
+        # for a non-streaming function but not for a streaming one, because the two
+        # paths copied the history at different points relative to running the
+        # function. See https://github.com/gradio-app/gradio/issues/10823.
+        def mutating(message, history):
+            history.append({"role": "assistant", "content": "INJECTED BY FN"})
+            return "reply"
+
+        def mutating_stream(message, history):
+            history.append({"role": "assistant", "content": "INJECTED BY FN"})
+            yield "reply"
+
+        expected = [
+            {"role": "user", "content": "hi"},
+            {"role": "assistant", "content": "reply"},
+        ]
+
+        original_history: list[Any] = []
+        _, non_streaming_history = await gr.ChatInterface(mutating)._submit_fn(
+            "hi", original_history
+        )
+        assert non_streaming_history == expected
+        assert original_history == []
+
+        stream = gr.ChatInterface(mutating_stream)._stream_fn("hi", original_history)
+        streamed = [chunk async for chunk in stream]
+        assert streamed[-1][1] == expected
+        assert original_history == []
+
     def test_custom_chatbot_with_events(self):
         with gr.Blocks() as demo:
             chatbot = gr.Chatbot()

test/test_external.py

@@ -1,10 +1,12 @@
+import copy
 import os
 import tempfile
 import textwrap
 from pathlib import Path
 from typing import cast
 from unittest.mock import MagicMock, patch
 
+import httpx
 import huggingface_hub
 import pytest
 
@@ -413,6 +415,98 @@ def test_load_chat_with_streaming(mock_openai):
     assert responses == ["Hello", "Hello World", "Hello World!"]
 
 
+def test_format_conversation_replays_text_files_as_text(tmp_path):
+    # A pasted long prompt arrives as a text file, which is inlined into the prompt on
+    # the turn it is sent. Once that turn was in the history it used to be re-sent as
+    # `image_url`, so every later message made a non-multimodal model reject the
+    # request. See https://github.com/gradio-app/gradio/issues/11331.
+    from gradio.external import format_conversation
+
+    text_file = tmp_path / "pasted_text.txt"
+    text_file.write_text("a very long pasted prompt")
+    image_file = tmp_path / "photo.png"
+    image_file.write_bytes(b"\x89PNG\r\n\x1a\n")
+    # `.R` is in `TEXT_FILE_EXTENSIONS` and `MultimodalTextbox` matches extensions
+    # case-insensitively, so both spellings have to be inlined as text here.
+    r_file = tmp_path / "plot.r"
+    r_file.write_text("plot(1:10)")
+
+    history = [
+        {
+            "role": "user",
+            "content": [{"type": "file", "file": {"path": str(text_file)}}],
+        },
+        {"role": "assistant", "content": [{"type": "text", "text": "ok"}]},
+        {
+            "role": "user",
+            "content": [{"type": "file", "file": {"path": str(image_file)}}],
+        },
+        {"role": "assistant", "content": [{"type": "text", "text": "ok"}]},
+        {
+            "role": "user",
+            "content": [{"type": "file", "file": {"path": str(r_file)}}],
+        },
+        {"role": "assistant", "content": [{"type": "text", "text": "ok"}]},
+    ]
+    history_before = copy.deepcopy(history)
+
+    conversation = format_conversation(history, "and now a short one")  # type: ignore
+
+    # the text file is inlined as text, the same as when it was first sent...
+    assert conversation[0]["content"] == [
+        {"type": "text", "text": "\n## pasted_text.txt\na very long pasted prompt"}
+    ]
+    # ...while an image is still sent as an image
+    image_content = conversation[2]["content"][0]
+    assert image_content["type"] == "image_url"
+    assert image_content["image_url"]["url"].startswith("data:image/png;base64,")
+
+    assert conversation[4]["content"] == [
+        {"type": "text", "text": "\n## plot.r\nplot(1:10)"}
+    ]
+
+    assert conversation[-1] == {
+        "role": "user",
+        "content": [{"type": "text", "text": "and now a short one"}],
+    }
+
+    # The messages passed in are the ones in `chatbot_state`, so rewriting them in place
+    # would replace the attachments in the transcript with what was sent to the model.
+    assert history == history_before
+
+
+def test_format_conversation_replays_remote_text_files_as_text(monkeypatch):
+    # A text file that is a URL rather than a local upload has to be inlined as text
+    # too, rather than base64-encoded into `image_url` like an image would be.
+    from gradio.external import format_conversation
+
+    def fake_get(url, *args, **kwargs):
+        assert url == "https://example.com/files/notes.txt"
+        return httpx.Response(
+            200, text="remote notes", request=httpx.Request("GET", url)
+        )
+
+    monkeypatch.setattr(httpx, "get", fake_get)
+
+    history = [
+        {
+            "role": "user",
+            "content": [
+                {
+                    "type": "file",
+                    "file": {"path": "https://example.com/files/notes.txt"},
+                }
+            ],
+        }
+    ]
+
+    conversation = format_conversation(history, "and now a short one")  # type: ignore
+
+    assert conversation[0]["content"] == [
+        {"type": "text", "text": "\n## notes.txt\nremote notes"}
+    ]
+
+
 def test_load_chat_textbox_override():
     from gradio import ChatInterface