feat(context-engine): lifecycle hooks - turn observation, request ass… - #47109
feat(context-engine): lifecycle hooks - turn observation, request ass…#47109huangxun375-stack wants to merge 1 commit into
Conversation
|
Duplicate of #15498 — both PRs implement the same ContextEngine lifecycle hooks feature (per-turn observation + request assembly), tracking feature request #23837. #15498 (after_turn / ingest_message hooks) is the earlier open PR. Marking this as a competing duplicate; maintainers can choose between the two approaches. |
…embly, lossless pre-compress snapshot, capability gating ContextEngine gains three optional lifecycle hooks (all no-op defaults, zero behavior change for the built-in compressor and existing engines): - on_turn_complete(messages, TurnInfo): finalized-turn observation in finalize_turn (ingest/index/memory without abusing compress()) - prepare_request_messages(messages, RequestContext) -> list|None: outbound-only request assembly before prompt caching; None = passthrough (prompt-cache prefix stays byte-stable); never written back to the canonical transcript - on_pre_compress(messages): lossless snapshot at the single compression chokepoint before compaction mutates the window Hosts gate every call on a ContextEngineCapabilities snapshot taken once at engine registration (capabilities() declared by the engine, fail-open to all-False defaults). engine_hook() wraps every dispatch: failures are logged and degrade to the original behavior. Review-fork agents are isolated from lifecycle side effects. Includes contract, integration, loop-path, and real-run_conversation e2e tests (40 tests).
a4b2aec to
e93da1b
Compare
@alt-glitch not a duplicateof #15498. Both touch ContextEngine and track #23837,
The request-assembly and pre-compress hooks simply don't exist in #15498, and its |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing a real ContextEngine limitation. The current diff needs isolation and validation fixes before it can safely provide the stated request-only, fail-open contract.
Problems
agent/conversation_loop.py:766passes canonicalmessagesto the request hook, andagent/turn_finalizer.py:426passes the same canonical list to observation. A hook can mutate persisted/live state even when it returnsNone; observation then affects the following memory sync atagent/turn_finalizer.py:443-449.agent/conversation_loop.py:787accepts every non-Nonereturn.[]replaces a valid request; non-list/non-dict values fail later during construction at:800-809.engine_hook()only catches exceptions from the hook, so this is not fail-open.agent/turn_finalizer.py:431-435forwards only three reconstructed usage fields, while the normalized host usage already includes cache and reasoning buckets atagent/conversation_loop.py:1880-1902.
Suggested changes
- Copy/isolate hook inputs and test that mutating hooks cannot alter canonical transcript or memory-sync input.
- Require a non-empty list of dicts before accepting an assembled view; otherwise preserve the original request.
- Forward the canonical per-turn usage dict and update the public ContextEngine plugin guide.
This is an automated hermes-sweeper review.
| _view = engine_hook( | ||
| _ce_engine, | ||
| "prepare_request_messages", | ||
| messages, |
There was a problem hiding this comment.
This passes the canonical mutable transcript to a hook whose contract says its view is request-only. An engine can mutate messages and return None, leaking changes into persistence despite the intended isolation. Pass an isolated message view and add a mutation regression.
| default=None, | ||
| logger=request_logger, | ||
| ) | ||
| if _view is not None: |
There was a problem hiding this comment.
A non-None invalid result does not fail open: [] replaces the request, while a non-list or non-dict member fails later in the loop. Accept only a non-empty list of message dicts; otherwise keep the original source and host injections.
| engine_hook( | ||
| _engine, | ||
| "on_turn_complete", | ||
| messages, |
There was a problem hiding this comment.
The observer receives the canonical list, then _sync_external_memory_for_turn() receives that same list below. A mutating observer can therefore alter the memory provider's input and the returned transcript. Pass an isolated snapshot instead.
| session_id=agent.session_id or "", | ||
| turn_id=turn_id, | ||
| turn_index=getattr(agent, "_user_turn_count", None), | ||
| usage={ |
There was a problem hiding this comment.
This drops the normalized cache and reasoning usage buckets already supplied to update_from_response(). Preserve and forward the canonical per-turn usage dict so an observation engine can assess the actual request rather than only three legacy counters.
Adds an optional, no-op-default select_context() hook to the ContextEngine ABC, called every turn after the request messages are assembled and before provider dispatch — independent of should_compress(). Lets an engine select or replace which context enters the prompt for a single request (retrieval, topic routing, role/branch switching) without mutating persisted history, removing the need to abuse should_compress()=True as a per-turn callback. The host call site (_apply_context_engine_selection) is fail-open: a missing hook, an exception, or an invalid return value leaves the assembled request untouched. Additive and non-breaking: the built-in compressor and every existing engine are unaffected. Consolidates the per-turn request-assembly surface proposed across NousResearch#41918, NousResearch#24949, NousResearch#47109, and NousResearch#50053 into one canonical hook (RFC NousResearch#36765). Related: NousResearch#36765 NousResearch#41918 NousResearch#24949 NousResearch#47109 NousResearch#50053 NousResearch#23837 NousResearch#25115 NousResearch#29370
Adds an optional, no-op-default select_context() hook to the ContextEngine ABC, called every turn after the request messages are assembled and before provider dispatch — independent of should_compress(). Lets an engine select or replace which context enters the prompt for a single request (retrieval, topic routing, role/branch switching) without mutating persisted history, removing the need to abuse should_compress()=True as a per-turn callback. The host call site (_apply_context_engine_selection) is fail-open: a missing hook, an exception, or an invalid return value leaves the assembled request untouched. Additive and non-breaking: the built-in compressor and every existing engine are unaffected. Consolidates the per-turn request-assembly surface proposed across #41918, Related: #36765 #41918 #24949 #47109 #50053 #23837 #25115 #29370
|
The lifecycle surface this PR proposed (request assembly + turn observation) has landed via salvage PR #70458 (from #51226, which consolidated the 4-PR cluster including this one — your PR is credited in its body and in the RFC discussion). The canonical verbs are Thanks @huangxun375-stack for converging on the same design independently — that convergence is what made the consolidation an easy call. If your engine needs a verb the landed pair doesn't cover, a focused follow-up against the new ABC surface is welcome. Closing as consolidated. |
Adds an optional, no-op-default select_context() hook to the ContextEngine ABC, called every turn after the request messages are assembled and before provider dispatch — independent of should_compress(). Lets an engine select or replace which context enters the prompt for a single request (retrieval, topic routing, role/branch switching) without mutating persisted history, removing the need to abuse should_compress()=True as a per-turn callback. The host call site (_apply_context_engine_selection) is fail-open: a missing hook, an exception, or an invalid return value leaves the assembled request untouched. Additive and non-breaking: the built-in compressor and every existing engine are unaffected. Consolidates the per-turn request-assembly surface proposed across NousResearch#41918, Related: NousResearch#36765 NousResearch#41918 NousResearch#24949 NousResearch#47109 NousResearch#50053 NousResearch#23837 NousResearch#25115 NousResearch#29370
What does this PR do?
This PR adds optional, capability-gated lifecycle hooks to
ContextEngineso external context engines can observe completed turns, assemble provider-bound request messages, and snapshot the full transcript before compression without abusingcompress()as a generic backdoor.Today
ContextEnginemainly models compression throughshould_compress()/compress(). That makes per-turn observation or pre-LLM context assembly difficult to express cleanly, and pushes engines toward treating request-time transforms as compaction events. This PR separates those concerns while preserving the existing built-in compressor behavior.This is intentionally separate from
MemoryProvider.MemoryProviderremains the right abstraction for simple long-term memory integrations: it can sync completed turns and inject recalled memory text. However, it cannot replace, trim, reorder, or rebuild the full provider-bound message list. Context engines need a different seam for systems that manage the whole outbound context window rather than only appending recalled memory.The new hooks are:
on_turn_complete(messages, TurnInfo): finalized-turn observation infinalize_turnfor ingest/index/memory workflows.prepare_request_messages(messages, RequestContext) -> list | None: outbound-only request assembly before provider dispatch;Nonemeans passthrough; returned views are never written back to the canonical transcript.on_pre_compress(messages): lossless snapshot opportunity at the single compression chokepoint before compaction mutates the window.Hosts gate every call on a
ContextEngineCapabilitiessnapshot taken once at engine registration. Hook dispatch is fail-open throughengine_hook(): failures are logged and degrade to the original behavior. Review-fork agents are isolated from lifecycle side effects.Related Issue
Related to #23837, #24949, #36765, and #41918.
Type of Change
Changes Made
agent/context_engine.pyTurnInfo,RequestContext,ContextEngineCapabilities, andengine_hook().on_turn_complete,prepare_request_messages, andon_pre_compresshooks.agent/conversation_loop.pyprepare_request_messagesbefore building provider-bound API messages.pre_llm_callcontext to the engine when it takes over assembly, avoiding double injection.agent/turn_finalizer.pyon_turn_completebefore external memory sync.run_agent.pyon_pre_compressat the compression chokepoint beforecompress()mutates the message window.agent/agent_init.pyandagent/background_review.pyrun_conversationE2E tests.How to Test
Targeted lifecycle suite result:
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — docstrings onlycli-config.yaml.exampleif I added/changed config keys — N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/AScreenshots / Logs