diff --git a/api/streaming.py b/api/streaming.py index 974ab92b..d1178df0 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -3120,7 +3120,14 @@ def _merge_display_messages_after_agent_result(previous_display, previous_contex if _has_context_only_turns: context_keys = [_message_identity(m) for m in previous_context] _backfilled = [] - _emitted = set() + # #3300 fix: track ONLY context rows we splice in, so the + # visible-display backbone is never suppressed. Sharing one set + # between context inserts and display rows (and _message_identity + # ignoring timestamps) dropped a legitimate second identical visible + # user turn. Display rows are always appended in order; a context + # row is backfilled only if it isn't already a display row and + # hasn't already been inserted. + _context_inserted = set() _cursor = 0 for _display_idx, _dmsg in enumerate(previous_display): _dkey = _message_identity(_dmsg) @@ -3132,9 +3139,9 @@ def _merge_display_messages_after_agent_result(previous_display, previous_contex for _k in range(_cursor, _j): _ckey = context_keys[_k] _cmsg = previous_context[_k] - if _ckey is not None and _ckey not in _emitted and not _is_context_compression_marker(_cmsg): + if _ckey is not None and _ckey not in _context_inserted and _ckey not in _display_id_set and not _is_context_compression_marker(_cmsg): _backfilled.append(copy.deepcopy(_cmsg)) - _emitted.add(_ckey) + _context_inserted.add(_ckey) _cursor = _j + 1 elif not any( _message_identity(_future_dmsg) in context_keys[_cursor:] @@ -3143,21 +3150,21 @@ def _merge_display_messages_after_agent_result(previous_display, previous_contex for _k in range(_cursor, len(context_keys)): _ckey = context_keys[_k] _cmsg = previous_context[_k] - if _ckey is not None and _ckey not in _emitted and not _is_context_compression_marker(_cmsg): + if _ckey is not None and _ckey not in _context_inserted and _ckey not in _display_id_set and not _is_context_compression_marker(_cmsg): _backfilled.append(copy.deepcopy(_cmsg)) - _emitted.add(_ckey) + _context_inserted.add(_ckey) _cursor = len(context_keys) - if _dkey not in _emitted: - _backfilled.append(_dmsg) - if _dkey is not None: - _emitted.add(_dkey) + # The display row is the visible backbone — always preserve it, + # in order, even when an earlier (identical-content) turn or a + # backfilled context row shares its timestamp-less identity. + _backfilled.append(_dmsg) while _cursor < len(context_keys): _ckey = context_keys[_cursor] _cmsg = previous_context[_cursor] _cursor += 1 - if _ckey is not None and _ckey not in _emitted and not _is_context_compression_marker(_cmsg): + if _ckey is not None and _ckey not in _context_inserted and _ckey not in _display_id_set and not _is_context_compression_marker(_cmsg): _backfilled.append(copy.deepcopy(_cmsg)) - _emitted.add(_ckey) + _context_inserted.add(_ckey) if len(_backfilled) > len(previous_display): logger.debug( "Backfilled %d context-only turns into previous_display (was %d, now %d)", diff --git a/tests/test_webui_gateway_chat_backend.py b/tests/test_webui_gateway_chat_backend.py index 44c5f924..9a87bbc1 100644 --- a/tests/test_webui_gateway_chat_backend.py +++ b/tests/test_webui_gateway_chat_backend.py @@ -452,6 +452,77 @@ def test_gateway_chat_worker_preserves_old_visible_turns_when_context_is_compact assert not any("context compaction" in m["content"] for m in saved.messages) +def test_gateway_chat_worker_keeps_repeated_identical_visible_turns(tmp_path, monkeypatch): + """#3300 regression (Codex gate): two identical visible user turns must BOTH + survive gateway finalization even when context-only rows are backfilled. + _message_identity ignores timestamps, so a shared identity must not let the + backfill dedup suppress the second visible turn.""" + session_dir = tmp_path / "sessions" + session_dir.mkdir() + monkeypatch.setattr(models, "SESSION_DIR", session_dir) + monkeypatch.setattr(models, "SESSION_INDEX_FILE", session_dir / "_index.json") + monkeypatch.setattr(models, "SESSIONS", OrderedDict()) + + class FakeResponse: + def __enter__(self): + return self + + def __exit__(self, exc_type, exc, tb): + return False + + def __iter__(self): + yield b'data: {"choices":[{"delta":{"content":"answer"}}]}\n\n' + yield b'data: [DONE]\n\n' + + monkeypatch.setenv("HERMES_WEBUI_GATEWAY_BASE_URL", "http://gateway.local") + monkeypatch.setattr(streaming, "_load_webui_prefill_context", lambda cfg: {"status": "not_configured", "source": "none", "label": "", "message_count": 0, "messages": []}) + monkeypatch.setattr(streaming, "_prefill_messages_with_webui_context", lambda ctx, cfg: []) + monkeypatch.setattr(gateway_chat.urllib.request, "urlopen", lambda req, timeout=0: FakeResponse()) + + s = new_session() + # Two identical visible "same" user turns surround a context-only gap that + # only lives in context_messages (plus a hidden compaction marker). + s.messages = [ + {"role": "user", "content": "same", "timestamp": 1.0}, + {"role": "assistant", "content": "first reply", "timestamp": 1.1}, + {"role": "user", "content": "same", "timestamp": 3.0}, + {"role": "user", "content": "new question", "timestamp": 4.0}, + ] + s.context_messages = [ + {"role": "assistant", "content": "[context compaction] hidden", "timestamp": 0.9}, + {"role": "user", "content": "same", "timestamp": 1.0}, + {"role": "assistant", "content": "first reply", "timestamp": 1.1}, + {"role": "user", "content": "context only gap", "timestamp": 2.0}, + {"role": "user", "content": "same", "timestamp": 3.0}, + ] + stream_id = "stream-gateway-repeated-identical-turns-test" + s.active_stream_id = stream_id + s.pending_user_message = "new question" + s.pending_attachments = [] + s.save() + STREAMS[stream_id] = create_stream_channel() + + gateway_chat._run_gateway_chat_streaming( + s.session_id, + "new question", + "test-model", + str(tmp_path), + stream_id, + [], + ) + + saved = models.get_session(s.session_id) + contents = [m["content"] for m in saved.messages] + # BOTH identical "same" visible turns must survive (the original bug dropped one). + assert contents.count("same") == 2, contents + # The context-only gap is backfilled into the visible transcript. + assert "context only gap" in contents + # The latest turn + reply are present. + assert contents[-2:] == ["new question", "answer"] + # No compaction marker leaks into the visible transcript. + assert not any("context compaction" in c for c in contents) + + def test_gateway_chat_worker_forwards_image_attachments_as_multimodal_parts(tmp_path, monkeypatch): session_dir = tmp_path / "sessions" session_dir.mkdir()