feat(context-engine): add request preparation hooks - #41918
Conversation
|
Small clarification on priority: this is not intended as a cosmetic plugin convenience. The current workaround for external context engines is to return This PR keeps existing behavior unchanged, but adds a fail-open native path for:
So the main value is reducing coupling and making context-engine plugins safer to implement. |
Expose optional post-turn observation and pre-request message preparation hooks so context engines can ingest finalized transcript snapshots and assemble request-only context without invoking host compression semantics.
093f764 to
71eb208
Compare
|
Looks ready from my side. The PR is currently blocked because required checks have not reported on the fork branch. Could a maintainer approve/run the workflows when convenient? |
|
RFC author of #36765 here — endorsing this direction. This is the selection-vs-compression split I argued for, landed as a concrete,
I have no stake in the naming or the exact param list — happy to defer to |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for supplying a narrow host contract and focused coverage. The requested hooks are still absent on current main, but one fail-open path needs tightening before salvage.
Problems
agent/conversation_loop.py:246-252accepts anylistreplacement. The new normalizer atagent/conversation_loop.py:260then calls.get()on each member, so a plugin return such as["bad"]raises instead of preserving the original request. Validate all members as message dicts (and reject an empty replacement) before accepting the result.- This is a public
ContextEngineAPI addition, butwebsite/docs/developer-guide/context-engine-plugin.md:93-101and:148-157still document the old optional-method and lifecycle contracts. The PR changes no documentation file.
Suggested changes
- Add malformed-return regression tests alongside the existing fail-open test: non-list, empty list, and a list containing a non-dict must each keep the original request.
- Document
on_turn_completeandprepare_request_messages, including the request-only persistence boundary.
Current main's request and finalization paths have moved (agent/conversation_loop.py:792-955, agent/turn_finalizer.py:30-46), so the DIRTY PR should be manually salvaged into those paths rather than applied mechanically.
Automated hermes-sweeper review.
|
|
||
| if replacement is None: | ||
| return request_messages, False | ||
| if not isinstance(replacement, list): |
There was a problem hiding this comment.
A list alone is not a safe replacement contract: ["bad"] passes this check, then _normalize_request_messages_for_api() calls .get() on the string and the request fails instead of failing open. Require a non-empty list whose members are message dicts (at minimum), otherwise log and return request_messages.
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 the post-turn observation verb as the companion to select_context(): an optional, no-op-default on_turn_complete() called once after the assistant/tool loop finishes, with the finalized transcript snapshot. Lets an engine ingest/index/summarize the completed turn to inform the next select_context(). Wired via _notify_context_engine_turn_complete() from turn_finalizer.finalize_turn(); fail-open, base no-op short-circuited so non-implementing engines (incl. the built-in compressor) pay nothing. This is the request-assembly + observation pair from NousResearch#41918; with this commit the PR fully subsumes NousResearch#41918's two hooks (prepare_request_messages -> select_context, on_turn_complete) rather than only the selection half. Co-authored-by: johnnykor82 <johnnykor82@users.noreply.github.com>
The on_turn_complete() observation hook is the engine's post-turn signal, so it should receive the completed turn's canonical token usage when the host has it, not a hardcoded None. Per @johnnykor82's NousResearch#41918 contract: the engine uses prompt/completion + cache_read/write/reasoning buckets to judge how large/expensive the selected context was before the next select_context(). - conversation_loop.py: stash the most recent provider response's usage_dict (the same canonical shape fed to update_from_response) on the agent as _last_turn_usage; reset to None at turn start so turns that never reach a provider response (early failure / interrupt) forward None, not a stale prior turn's usage. - turn_finalizer.py: forward agent._last_turn_usage instead of usage=None. - context_engine.py: document the usage param contract on the ABC hook. - tests: cover both ends through the real finalize_turn path — completed turn forwards the full canonical bucket set intact; no-response turn forwards None. Co-authored-by: johnnykor82 <johnnykor82@users.noreply.github.com>
… public hooks - _apply_context_engine_selection: reject an empty list. all([]) is True, so a [] returned by a failing/buggy engine previously replaced a valid request with an empty message list the downstream sanitizers can't restore; now it falls open to the unmodified request (honors the fail-open contract). Thanks @johnnykor82 for catching this on NousResearch#41918's review. - test: empty list keeps the original request (fail-open regression). - docs: document select_context()/on_turn_complete() in the public context-engine plugin guide (were still describing only the old contract).
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
Adds the post-turn observation verb as the companion to select_context(): an optional, no-op-default on_turn_complete() called once after the assistant/tool loop finishes, with the finalized transcript snapshot. Lets an engine ingest/index/summarize the completed turn to inform the next select_context(). Wired via _notify_context_engine_turn_complete() from turn_finalizer.finalize_turn(); fail-open, base no-op short-circuited so non-implementing engines (incl. the built-in compressor) pay nothing. This is the request-assembly + observation pair from #41918; with this commit the PR fully subsumes #41918's two hooks (prepare_request_messages -> select_context, on_turn_complete) rather than only the selection half. Co-authored-by: johnnykor82 <johnnykor82@users.noreply.github.com>
The on_turn_complete() observation hook is the engine's post-turn signal, so it should receive the completed turn's canonical token usage when the host has it, not a hardcoded None. Per @johnnykor82's #41918 contract: the engine uses prompt/completion + cache_read/write/reasoning buckets to judge how large/expensive the selected context was before the next select_context(). - conversation_loop.py: stash the most recent provider response's usage_dict (the same canonical shape fed to update_from_response) on the agent as _last_turn_usage; reset to None at turn start so turns that never reach a provider response (early failure / interrupt) forward None, not a stale prior turn's usage. - turn_finalizer.py: forward agent._last_turn_usage instead of usage=None. - context_engine.py: document the usage param contract on the ABC hook. - tests: cover both ends through the real finalize_turn path — completed turn forwards the full canonical bucket set intact; no-response turn forwards None. Co-authored-by: johnnykor82 <johnnykor82@users.noreply.github.com>
… public hooks - _apply_context_engine_selection: reject an empty list. all([]) is True, so a [] returned by a failing/buggy engine previously replaced a valid request with an empty message list the downstream sanitizers can't restore; now it falls open to the unmodified request (honors the fail-open contract). Thanks @johnnykor82 for catching this on #41918's review. - test: empty list keeps the original request (fail-open regression). - docs: document select_context()/on_turn_complete() in the public context-engine plugin guide (were still describing only the old contract).
|
The request-preparation + turn-observation surface this PR proposed has landed via salvage PR #70458 (from #51226, the RFC #36765 consolidation): Beyond the design, your review on #51226 caught two real defects (the |
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
Adds the post-turn observation verb as the companion to select_context(): an optional, no-op-default on_turn_complete() called once after the assistant/tool loop finishes, with the finalized transcript snapshot. Lets an engine ingest/index/summarize the completed turn to inform the next select_context(). Wired via _notify_context_engine_turn_complete() from turn_finalizer.finalize_turn(); fail-open, base no-op short-circuited so non-implementing engines (incl. the built-in compressor) pay nothing. This is the request-assembly + observation pair from NousResearch#41918; with this commit the PR fully subsumes NousResearch#41918's two hooks (prepare_request_messages -> select_context, on_turn_complete) rather than only the selection half. Co-authored-by: johnnykor82 <johnnykor82@users.noreply.github.com>
The on_turn_complete() observation hook is the engine's post-turn signal, so it should receive the completed turn's canonical token usage when the host has it, not a hardcoded None. Per @johnnykor82's NousResearch#41918 contract: the engine uses prompt/completion + cache_read/write/reasoning buckets to judge how large/expensive the selected context was before the next select_context(). - conversation_loop.py: stash the most recent provider response's usage_dict (the same canonical shape fed to update_from_response) on the agent as _last_turn_usage; reset to None at turn start so turns that never reach a provider response (early failure / interrupt) forward None, not a stale prior turn's usage. - turn_finalizer.py: forward agent._last_turn_usage instead of usage=None. - context_engine.py: document the usage param contract on the ABC hook. - tests: cover both ends through the real finalize_turn path — completed turn forwards the full canonical bucket set intact; no-response turn forwards None. Co-authored-by: johnnykor82 <johnnykor82@users.noreply.github.com>
… public hooks - _apply_context_engine_selection: reject an empty list. all([]) is True, so a [] returned by a failing/buggy engine previously replaced a valid request with an empty message list the downstream sanitizers can't restore; now it falls open to the unmodified request (honors the fail-open contract). Thanks @johnnykor82 for catching this on NousResearch#41918's review. - test: empty list keeps the original request (fail-open regression). - docs: document select_context()/on_turn_complete() in the public context-engine plugin guide (were still describing only the old contract).
What does this PR do?
Adds two optional
ContextEnginehooks for memory/context-management plugins:on_turn_complete(...)observes a finalized turn transcript snapshot after a user turn completes.prepare_request_messages(...)can replace provider request messages for a single API call without mutating persisted conversation history.This gives external context engines a native path for ingestion and request-only context assembly without forcing
should_compress() == Truejust to receive message history. Existing engines keep their current behavior because both hooks default to no-op and host calls fail open.Related/complementary work: #15498 proposes per-message/after-turn lifecycle hooks. This PR is narrower around finalized-turn observation and the pre-provider request replacement surface needed for retrieval/context assembly.
Related Issue
Related to #23837, #25115, #36765, and #29370.
Type of Change
Changes Made
agent/context_engine.py: adds optionalon_turn_complete(...)andprepare_request_messages(...)no-op methods to the ContextEngine contract.agent/conversation_loop.py: calls the post-turn hook with shallow-copied transcript and usage metadata; calls the request hook before provider cache-control annotations; validates hook return values and fails open on errors.tests/agent/test_context_engine.py: covers default no-op hook behavior.tests/agent/test_context_engine_host_contract.py: covers metadata forwarding, copy semantics, request-only replacement, and fail-open behavior.tests/run_agent/test_run_agent.py: covers full conversation-loop behavior, request-only persistence semantics, and prompt-cache ordering.How to Test
Focused companion-plugin hook checks:
Result:
6 passed.Same focused checks against the isolated runtime-copy plugin:
Result:
6 passed.Focused Hermes host-contract checks:
Result:
27 passed, 1 warning(audioopdeprecation fromdiscord/player.py).Additional checks:
Result: both passed.
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AScreenshots / Logs
No UI changes.