From 13fe08d7e939d8b16e976733471f981995e3a844 Mon Sep 17 00:00:00 2001
From: Teknium <127238744+teknium1@users.noreply.github.com>
Date: Thu, 23 Jul 2026 18:45:24 -0700
Subject: [PATCH] fix(context-engine): short-circuit the inherited no-op
 select_context before any per-request work
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Verification follow-up for the #51226 salvage: the host call site guarded
select_context with hasattr(), but the ABC defines a default on every
engine, so the built-in ContextCompressor (and any non-implementing
engine) still paid per-request shallow copies of the conversation
history plus a hook call on every provider request. Identity-check the
bound method against ContextEngine.select_context and return the
request untouched — mirroring the existing base-method short-circuit in
_notify_context_engine_turn_complete — so the default path does zero
work, not just produces an identical result.

Adds two pins: the base no-op is never invoked (patched-to-raise base
stays silent), and ContextCompressor.__dict__ contains neither new verb.

Also registers the contributor email mapping for @chaos-xxl.
---
 agent/conversation_loop.py                    | 14 +++++++
 contributors/emails/chaosxinglong@gmail.com   |  1 +
 .../test_context_engine_select_context.py     | 42 +++++++++++++++++++
 3 files changed, 57 insertions(+)
 create mode 100644 contributors/emails/chaosxinglong@gmail.com

diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py
index ef4b48cc12603..a234213e32caf 100644
--- a/agent/conversation_loop.py
+++ b/agent/conversation_loop.py
@@ -739,6 +739,20 @@ def _apply_context_engine_selection(
     if engine is None or not hasattr(engine, "select_context"):
         return api_messages
 
+    # Skip the no-op base implementation so non-implementing engines —
+    # including the built-in ContextCompressor — pay nothing per request:
+    # no history copies below, no call. ``hasattr`` alone is not enough,
+    # because the ABC defines a default ``select_context`` that every engine
+    # inherits. Mirrors the base-method short-circuit in
+    # ``_notify_context_engine_turn_complete``. Lazy import avoids any import
+    # cycle with agent.context_engine.
+    try:
+        from agent.context_engine import ContextEngine as _CE
+        if getattr(engine.select_context, "__func__", None) is _CE.select_context:
+            return api_messages
+    except Exception:
+        pass
+
     session_label = getattr(agent, "session_id", None) or "-"
     # Pass shallow copies of the reference-only inputs so an engine that
     # mutates them in place cannot alter persisted transcript state. Only
diff --git a/contributors/emails/chaosxinglong@gmail.com b/contributors/emails/chaosxinglong@gmail.com
new file mode 100644
index 0000000000000..dac570b173992
--- /dev/null
+++ b/contributors/emails/chaosxinglong@gmail.com
@@ -0,0 +1 @@
+chaos-xxl
diff --git a/tests/agent/test_context_engine_select_context.py b/tests/agent/test_context_engine_select_context.py
index 350acc360f27f..c8523943708e0 100644
--- a/tests/agent/test_context_engine_select_context.py
+++ b/tests/agent/test_context_engine_select_context.py
@@ -89,6 +89,48 @@ def test_none_return_leaves_request_unchanged():
     assert out is REQUEST
 
 
+def test_base_noop_select_context_is_short_circuited_not_called():
+    """Non-implementing engines skip the hook entirely (no call, no copies).
+
+    The built-in ContextCompressor — and any engine that merely inherits the
+    ABC default — must keep the default request path byte-identical AND pay
+    nothing per request. ``hasattr`` alone cannot distinguish "inherits the
+    no-op default" from "implements the hook" because the ABC defines
+    ``select_context`` on every engine; the host therefore identity-checks the
+    bound method against ``ContextEngine.select_context`` and short-circuits
+    WITHOUT calling it or building the shallow reference copies. This pins
+    that: even a base implementation patched to raise is never invoked.
+    """
+    from unittest.mock import patch as _patch
+
+    def _explode(self, request_messages, **kwargs):
+        raise AssertionError("base select_context must not be invoked")
+
+    engine = _MinimalEngine()  # inherits the ABC default
+    agent = _agent_with(engine)
+    logger = MagicMock()
+    with _patch.object(ContextEngine, "select_context", _explode):
+        out = _apply_context_engine_selection(
+            agent, REQUEST, HISTORY, HISTORY[-1], logger=logger
+        )
+    assert out is REQUEST
+    assert not logger.warning.called
+
+
+def test_builtin_compressor_inherits_base_select_context():
+    """The built-in ContextCompressor must NOT implement the new verbs.
+
+    Guards the default-path byte-identity contract: if someone overrides
+    ``select_context`` / ``on_turn_complete`` on ContextCompressor, the host
+    short-circuits no longer skip it and the default request pipeline gains a
+    per-request call — update this pin only together with that decision.
+    """
+    from agent.context_compressor import ContextCompressor
+
+    assert "select_context" not in ContextCompressor.__dict__
+    assert "on_turn_complete" not in ContextCompressor.__dict__
+
+
 def test_missing_hook_leaves_request_unchanged():
     """An engine without select_context (older/stub base) is a no-op."""
     engine = object()  # no select_context attribute