fix(context-engine): short-circuit the inherited no-op select_context before any per-request work
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.
This commit is contained in:
parent
56e00f4ca1
commit
13fe08d7e9
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -0,0 +1 @@
|
|||
chaos-xxl
|
||||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue