fix(context): bound session recall payloads - #55640
Conversation
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: LGTM
Adds profile forwarding to session_search tool calls and bounds session recall payloads with metadata constants.
✅ Looks Good
- Clean implementation: profile parameter forwarded from agent to session_search
- Good test coverage with 2 new tests (sequential and invoke paths)
- Schema description updated to document bounded payloads
- Well-scoped fix for cross-profile session search
Reviewed by Hermes Agent
2271c09 to
a0f492c
Compare
|
Hit this exact problem before finding this PR. Data point in case it helps review. On an instance with a large session history, a single session_search discover-mode call (query, limit=5) returned a ~49400-char tool result. ~22,200 chars was actual message content. The _MESSAGE_CONTENT_MAX_CHARS / _TOOL_CALL_ARGUMENTS_MAX_CHARS / _TOOL_CALLS_MAX_ITEMS caps here would have cut that call from ~49k chars to a small fraction of that. One gap: _shape_tool_calls_for_recall copies each call dict (shaped = dict(call)) and only reshapes function.arguments / top-level arguments. id / call_id / response_item_id still pass through unchanged. Not as big a deal since arguments and count are capped which was the main bloat. Possible follow-up though. Would like to see this merged. |
a0f492c to
e3cfc29
Compare
e3cfc29 to
5ac7fef
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing a real recall-context bloat path. Current main still returns raw recalled content and tool-call blobs through tools/session_search_tool.py:123-141 and discovery bookends/windows at :588-603.
Problems
tools/session_search_tool.py:153misses merged compaction summaries. Current compaction intentionally puts the summary prefix after_MERGED_SUMMARY_DELIMITER(agent/context_compressor.py:138-145,2259-2270), so this supported form is not omitted.tools/session_search_tool.py:278copies retained call dictionaries wholesale. Onlyargumentsis bounded, so an oversizedresponse_item_idor other provider field still defeats the stated bounded-payload guarantee.website/docs/user-guide/sessions.md:528still promises “no truncation.”
Suggested changes
- Reuse or precisely mirror the compressor's merged/legacy summary detection and add its regression shape.
- Shape tool calls with an explicit bounded field set (or cap every retained field), plus a non-arguments oversized-field test.
- Update the sessions guide for bounded recall output.
Automated hermes-sweeper review.
| return len(str(content)) | ||
|
|
||
|
|
||
| def _is_compaction_summary_content(content: Any) -> bool: |
There was a problem hiding this comment.
This only recognizes a summary when its prefix starts the string. Current compaction can merge a prior tail before the summary and place the prefix after _MERGED_SUMMARY_DELIMITER (agent/context_compressor.py:138-145, 2259-2270), so that supported summary form is returned rather than omitted. Please match the compressor's detection semantics and add a merged-summary regression.
| shaped_calls.append(call) | ||
| continue | ||
|
|
||
| shaped = dict(call) |
There was a problem hiding this comment.
Copying the whole provider call object leaves every non-arguments field unbounded. A large response_item_id or another provider-specific field survives the count/arguments caps, so the recall payload is still not bounded. Please use an explicit shaped field set or cap all retained fields, with a regression test.
5ac7fef to
b45bac2
Compare
b45bac2 to
3a39f01
Compare
|
enhanced with more tests and self reviews 🫡 |
…n-recall-payloads # Conflicts: # agent/context_compressor.py # tools/session_search_tool.py
…n-recall-payloads
session_search replays raw message content from past sessions verbatim, with no scan and no untrusted-content wrapping. A message that carries an injection payload -- a poisoned web page quoted earlier, a pasted phishing email, a Brainworm-style payload from any prior turn -- gets served back into the model's context as plain data on a later query, unmarked. Every other tool that returns attacker-controllable content (web_extract, web_search, browser_*, mcp_*) already gets wrapped in <untrusted_tool_result> delimiters via make_tool_result_message(), telling the model to treat the content as data, not instructions. session_search was simply missing from that list. Add it.
|
resimplified this branch to stay on original target |
Skywind5487
left a comment
There was a problem hiding this comment.
Research note for #45/#47: the current PR body still says lineage traversal has “depth and lookup budgets plus path compression” and “stops once the requested result limit is satisfied”, but the current head diff (d0e5a364...) no longer contains lineage traversal/path-compression changes; its present tools/session_search_tool.py diff is summary detection/content bounding only. Please do not treat #55640's current head as an implementation of the path-compressed contender without checking an earlier commit/revision. This is just provenance clarification, not a request to change the PR.
What does this PR do?
Bounds
session_searchoutput before recalled history is injected back into the active model context.The original bug is recursive context bloat: one recall call can import prior messages, compaction summaries, tool metadata, and lineage/bookend payloads into the current model context. If that recalled result is itself large or contains stale generated summaries, it can consume another context window and reactivate obsolete task state.
This PR keeps recall source-linked and navigable while bounding every returned layer:
The result preserves session IDs, message IDs, roles, timestamps, tool names, source references, and navigation metadata so an agent can request a narrower scroll/read rather than receiving an unbounded transcript copy.
Related Issue
session_searchdiscovery can rehydrate huge compaction summaries through bookends.Related PRs:
session_searchbookends and caps content length intools/session_search_tool.py/tests/tools/test_session_search.py. This PR is broader: it also handles discovery snippets, structured content, nested and top-level tool-call arguments, schema wording andprofileforwarding through built-in execution paths.agent/context_compressor.py; not duplicate because it prevents duplicate persisted handoffs at compression time rather than boundingsession_searchrecall output.Type of Change
Changes Made
tools/session_search_tool.pyagent/context_compressor.pyhermes_state.pytests/agent/test_context_compressor.pytests/tools/test_session_search.pywebsite/docs/user-guide/sessions.mdHow to Test
Final local verification:
tests/agent/test_context_compressor.py: 162 passedtests/tools/test_session_search.py: 79 passedgit diff --checkpassedGitHub Actions on the final head also pass:
Manual behavior checks:
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/AFor New Skills
N/A
Screenshots / Logs